search

exercism / v2-configlet

Tool to assist in managing Exercism language tracks.
MIT License
16 stars 23 forks source link

fetch-configlet: set pipefail shell option #166

Closed guygastineau closed 5 years ago

guygastineau commented 5 years ago

So here it is with the pipefail option set.

I usually stay away from the errexit option, but it occurs to me that it could be beneficial in this case. If we also set the the errexit option then I believe this script will exit with an appropriate error message even if the subprocess from the command substitution in the assignment to VERSION fails. This does seem like the best behavior.

Therefore, please let me know if there is agreement that errexit should also be set :smile:

PS. I have a style concern about the terminators in the case statements. This is outside of this PR's scope, but after this is merged I will open a PR that presents changes to the case style (specifically to make it conform more closely to some of the most accepted Bash style guides on the web ;) )

kytrinyx commented 5 years ago

Therefore, please let me know if there is agreement that errexit should also be set

LOL. You're assuming so much about my knowledge of exit codes. I'm in the "seems fine" group :-)

kytrinyx commented 5 years ago

I have a style concern

I would gladly accept style concern fixes.

Once you've updated that, I'll update the various track builds to get this file from this repo, instead of storing it, I think.

guygastineau commented 5 years ago

Cool. I didn't set the errexit option yet, but that is trivial. It will literally requires the addition of 1 character, so I will just include it in the style PR if that is acceptable 😀

I am taking my mom to the shoe store, so it'll be later today before I get the next PR up here 😎

kytrinyx commented 5 years ago

I will just include it in the style PR if that is acceptable

Totally! And I see that shoe mission is complete, and style PR is up :-) I'll go look at that now.