-
-
Notifications
You must be signed in to change notification settings - Fork 398
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
fetch_configlet: update, and remove duplicate script #1218
Conversation
Commit eff52e6 "Update bin/fetch-configlet script" added a new up-to-date fetch script, rather than modifying the existing one. Remove the new script, and update the old one with the same upstream changes [1][2][3][4]: - fetch-configlet: allow running from the bin directory - fetch-configlet: add comment with upstream location - fetch-configlet: remove a hard line wrap - fetch-configlet: improve style slightly [1] exercism/configlet@3815d7a11ece [2] exercism/configlet@d304fa5f4291 [3] exercism/configlet@9c69c4d40827 [4] exercism/configlet@3b4b00983ac6
Thank you for contributing to Based on the files changed in this PR, it would be good to pay attention to the following details when reviewing the PR:
Automated comment created by PR Commenter 🤖. |
I would prefer the I don't feel strongly about having the script be extensionless, though. That's just an artifact of an earlier choice that I made without having strong feelings about the options. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with merging this and then changing the extensions in a follow-on PR, if that is the outcome of the discussion.
Sounds good to me. I think we intended to get around to doing that. If we change the filename in this repo (which we would do if we add this file to Lines 4 to 5 in eff52e6
|
OK then, I'll merge this and expect a follow up PR with a name change (I do have a slight preference for having an extension). |
Commit eff52e6 "Update bin/fetch-configlet script" added a new up-to-date fetch script, rather than modifying the existing one.
Remove the new script, and update the old one with the same upstream changes:
3815d7a
fetch-configlet: allow running from the bin directoryd304fa5
fetch-configlet: add comment with upstream location9c69c4d
fetch-configlet: remove a hard line wrap3b4b009
fetch-configlet: improve style slightlyFollow-up from #1217.
Alternatively, we could also remove the
.sh
script instead. But if we prefer the extensionless name, maybe it's better to merge this PR first, then remove all.sh
from scripts in this repo as a follow-up PR.