chshersh / tool-sync

🧰 Download pre-built binaries of all your favourite tools with a single command
https://crates.io/crates/tool-sync
Mozilla Public License 2.0
69 stars 16 forks source link

[#82] Adds proxy as a flag and config option #121

Closed MitchellBerend closed 1 year ago

MitchellBerend commented 1 year ago

Resolves #82

This pr adds a proxy flag and a proxy option to the tool.toml, if both are supplied the proxy flag supersedes the config option.

Additional tasks

MitchellBerend commented 1 year ago

@chshersh @maku can you both check this out? This implementation functionally works for me, but I don't see a way to actually test this.

chshersh commented 1 year ago

Thanks for working on this, @MitchellBerend! 👏🏻

Just a heads-up, I have more suggestions about changing the code. But before putting more work into this, I'd like to have a confirmation that the feature works. I don't use any proxy and once this feature is merged, it will probably remain in the code forever and require maintenance.

Spending more of my free time maintaining features I don't even use is not something I'm eager to do 😌 And it's even worse if nobody else is going to use them either 😢

maku commented 1 year ago

@MitchellBerend @chshersh sorry for delay, recently I worked mostly remote where I do not have a proxy server. But today I tried the implementation (I cloned https://github.com/MitchellBerend/tool-sync).

For my use case it worked perfectly. (I am using a proxy without auth...)

chshersh commented 1 year ago

@maku Thanks for checking the implementation!

In that case, I'm happy to accept the patch 🙂 I'll get around providing more feedback on the implementation once I have time 👌🏻

MitchellBerend commented 1 year ago

@maku what os are you on? I would also like to know if this works on windows.

Should auth not be supported on this? My use case also does not require auth so I did not implement it. Support for this can always be added later when needed so maybe this is work that doesn't need to be done right now.

maku commented 1 year ago

@MitchellBerend I tried it on windows

chshersh commented 1 year ago

And here is the issue with the future improvements suggestions: