Conversation
- Added config for selecting most ranked server when choosing location - Initial commit
|
Thank you for your contribution! 🎉 🔔 @0x46616c6b @SebKranz you might want to have a look. You can use this guide to learn how to check out the Pull Request locally in order to test it. Due to our current reduced availability, the initial review may take up to 10-15 business days |
There was a problem hiding this comment.
PR Summary
This PR enhances the Mullvad VPN extension with server ranking capabilities and improved location selection functionality.
- Added server ranking persistence using Cache API with ability to reset rankings for both locations and servers
- Implemented preference-based server selection through new
selectByRankingpreference - Added detailed metadata display showing country/city codes in location list
- Fixed CHANGELOG.md to use
{PR_MERGE_DATE}template string instead of hardcoded date - Improved error handling by utilizing
showFailureToastfrom@raycast/utils
💡 (4/5) You can add custom instructions or style guidelines for the bot here!
2 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile
| setTopServer(sortedServers[0]); | ||
| execSync(`mullvad relay set location ${server.id}`); |
There was a problem hiding this comment.
logic: Incorrect command format - server.id alone won't work. Should be ${countryCode} ${cityCode} ${server.id}
| setTopServer(sortedServers[0]); | |
| execSync(`mullvad relay set location ${server.id}`); | |
| setTopServer(sortedServers[0]); | |
| execSync(`mullvad relay set location ${location.countryCode} ${location.cityCode} ${server.id}`); |
| if (rankingCacheRef.current.get(`ranked-servers-${location.id}`).size > 1) { | ||
| setTopServer(sortedServers[0]); | ||
| } else { | ||
| setTopServer({ id: "" }); | ||
| } |
There was a problem hiding this comment.
logic: Race condition possible here. sortedServers[0] might be stale after resetRanking. Should fetch fresh sorted servers list
| let topServerID = ""; | ||
| if (selectByRanking) topServerID = rankingCacheRef.current.get("top-servers")?.[location.id] || ""; | ||
| execSync(`mullvad relay set location ${countryCode} ${cityCode} ${topServerID}`); |
There was a problem hiding this comment.
style: Consider wrapping execSync in try-catch to handle potential CLI execution errors
pernielsentikaer
left a comment
There was a problem hiding this comment.
Hi 👋
Looks good to me, approved 🔥
|
Published to the Raycast Store: |
|
🎉 🎉 🎉 We've rewarded your Raycast account with some credits. You will soon be able to exchange them for some swag. |
Description
I made a mistake and didn't push the correct code, so this is the actual update, sorry!
Screencast
Checklist
npm run buildand tested this distribution build in Raycastassetsfolder are used by the extension itselfREADMEare placed outside of themetadatafolder