Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve venv script #10

Open
Irubataru opened this issue Feb 8, 2024 · 0 comments
Open

Improve venv script #10

Irubataru opened this issue Feb 8, 2024 · 0 comments

Comments

@Irubataru
Copy link
Owner

Based on feedback from Alessandro

  • I prefer /usr/bin/env in the shebang. AFAIK it is more portable across different Unix OS.
  • This is minor, but lately I changed mind about if [[ -n "${VIRTUAL_ENV}" ]]. It is for sure correct, but I find if [[ "${VIRTUAL_ENV}” != '' ]] simply more direct and easier to read, avoiding some brain gymnastic.
  • I would use set -u in scripts, too. I mean, your editor might be advanced enough to spot typos, but accessing unset variables in Bash is something I got bitten by a couple of times. Sure, nounset option has drawbacks e.g. in functions interfaces/design but it is easy to go around them with parameter expansion.
  • These three lines look odd to me. Maybe I miss the point, but is your mkdir doing anything at all?
  • Another voice probably outside the box: I never really used getopt. I do not really see a reason for using getopt. Everything it offers (than I am aware of and ever needed) is easily implementable in bash. And getopt versions on BSD, Debian, GNU and so on are different. I simply avoid this headache. What you have from line 243 on is basically a parser of the arguments already and I implement there all my parsing myself.
  • Furthermore, the eval on line 242 is breaking parsing of possible command line arguments with spaces in them, isn’t it? I am not saying you will ever ending having a space in a filename… ;) What is that eval for?
  • You have this pattern repeated some times and you might delegate it to a function that takes the function to call as first argument and forward the remaining argument to the function to be called. Proof of concept.
  • I am also not sure what you would like to have here. Maybe something to discuss in next email.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant