search

ProtonVPN / protonvpn-cli

Legacy protonvpn-cli: ProtonVPN Command-Line Tool for Linux and macOS. This has been superseded by https://github.com/ProtonVPN/protonvpn-cli-ng
Other
607 stars 98 forks source link

Use `-n` instead of `! -z` in tests #177

Closed vitorgalvao closed 5 years ago

vitorgalvao commented 5 years ago

Functionally equivalent and easier to parse. Check if it has something and not if it is not empty (double negative).

There a few more quick wins to be had in the script, but I’d rather submit one at a time, so as to not clash with other PRs.

xilopaint commented 5 years ago

Hey @vitorgalvao, you could give a great contribution by creating a Homebrew formula for the CLI. 😀

vitorgalvao commented 5 years ago

Hey @vitorgalvao, you could give a great contribution by creating a Homebrew formula for the CLI.

We’ll see how it goes. I don’t even yet know if the contributions I’m planning here will be reviewed/accepted.

I’m speaking only for myself, and not on behalf of the other Homebrew maintainers, but most of my time is already spent reviewing PRs and issues. If a user wants to see some software added to Homebrew, they are expected to do the legwork and submit a PR, at which point we can help making it the best it can be for inclusion.

This helps save maintainer time, which is good as we don’t get as overwhelmed. But it also ensures we only add (and thus only have to maintain) software our users really want. We have tons of contributors, and so many new additions we need rules for things we have to refuse. If no one is sufficiently interested in a piece of software to try to submit a formula or cask, that’s a good indication it isn’t that popular with our user base.

xilopaint commented 5 years ago

If no one is sufficiently interested in a piece of software to try to submit a formula or cask, that’s a good indication it isn’t that popular with our user base.

It makes sense, but the formula was already requested in this repo (#22 #27).

The only reason I didn't submit a PR to Homebrew Core yet is I have no knowledge on Ruby. Yet, at some point I will get my hands dirty to learn the basics of the language enough to create a formula.

vitorgalvao commented 5 years ago

but the formula was already requested in this repo

I said submitted, not requested. The latter can be a comment in passing; the former shows commitment.

The only reason I didn't submit a PR to Homebrew Core yet is I have no knowledge on Ruby.

When I started submitting to Homebrew Cask, and for a long time after I became a maintainer, I had close to no knowledge of ruby. Several other maintainers are also not rubyists. You don’t need any ruby knowledge to make a formula or a cask. You don’t even need to fully understand the DSL; read the documentation and use another formula/cask as the basis for your own. Try to find some software that is similar in contents (I sent you a reference when we first talked about this on Twitter) and change it to fit your case. Submit that, and a maintainer will help iron the kinks.

kaplun commented 5 years ago

Hi @vitorgalvao this looks good to me :+1:

kaplun commented 5 years ago

P.s. I am taking over from @mazen160 in integrating code. Thanks a lot for your contribution.