willeccles / cpm

🌵 A wrapper for package managers to make them consistent for those of us who are lazy.
MIT License
69 stars 13 forks source link

force a pm function to be used with _pm=_function cpm & shellcheck QA fixes #31

Closed xgqt closed 3 years ago

willeccles commented 3 years ago

@6gk review this please thanks

xgqt commented 3 years ago

Now that I look at my commit I think I worded it wrongly, just to clarify: with the latest commit it will be possible to force a specific pm function to be used with passing a _pm env var, ie.: _pm=_guix cpm list.

Of course it would be better to do pm=guix cpm list or cmp -p guix list but it would be spaghetti code w/o renaming pm functions.

eepykate commented 3 years ago

Of course it would be better to do pm=guix cpm list or cmp -p guix list but it would be spaghetti code w/o renaming pm functions.

No, it wouldn't.

It would be possible to do

pm=guix cpm by using

if [ "$pm" ] && has "_$pm"; then
  "_$pm" "$@"

(command -v (which has uses) includes functions. That's to stop it trying to run contents of unrelated variable)

xgqt commented 3 years ago

update

eepykate commented 3 years ago

(also cd || return; printf is functionally identical to cd && printf but longer)

xgqt commented 3 years ago

(also cd || return; printf is functionally identical to cd && printf but longer)

https://github.com/willeccles/cpm/pull/31#discussion_r625677468

there you wanted it on separate lines

ok, will do in one

eepykate commented 3 years ago

there you wanted it on separate lines

:shrug: Both are possible, it's just a stylistic choice.

command1 &&
command2
command1 && command2

are both valid.

And I was commenting on the code itself, and not the lines.

foxsouns commented 3 years ago

Hey: while we're here, the line at 327 overflows the 80 character limit that Will set in CONTRIBUTING.md.

willeccles commented 3 years ago

Hey: while we're here, the line at 327 overflows the 80 character limit that Will set in CONTRIBUTING.md.

In this case I will let it slide, I might update it in the future but I dislike breaking strings. It's okay to go over 80 here unless you can stay under 80 and make it look decent.

willeccles commented 3 years ago

Once @6gk and @xgqt have finished up with the review and gk gives me the thumbs up, I'll merge.

eepykate commented 3 years ago

@willeccles Yeah I think it's good now.