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

contrithanks.sh: fix "useless cat", make it update THANKS in-place #16448

Closed
wants to merge 3 commits into from

Conversation

bagder
Copy link
Member

@bagder bagder commented Feb 24, 2025

cat ./docs/THANKS | sed 's/ github/ github/i'
    ^-----------^ SC2002 (style): Useless cat. Consider 'cmd < file | ..' or 'cmd file | ..' instead.

cat ./docs/THANKS | sed 's/ github/ github/i'
    ^-----------^ SC2002 (style): Useless cat. Consider 'cmd < file | ..' or 'cmd file | ..' instead.
@testclutch
Copy link

Analysis of PR #16448 at 7e5f2bbe:

Test 1906 failed, which has NOT been flaky recently, so there could be a real issue in this PR.

Generated by Testclutch

instead of generating the update on stdout

still needs a manual sort
This has the small side-effect that it sorts slightly differen than the
previously used sort function (emacs).

I think this is a better sort and over all it makes it more convenient
to use the script as it removes a manual step.
@bagder bagder changed the title contrithanks.sh: fix "useless cat" contrithanks.sh: fix "useless cat", make it update THANKS in-place Feb 24, 2025
@bagder
Copy link
Member Author

bagder commented Feb 24, 2025

I made the script sort the list of names in place too using 'sort', instead of the old "manual sort" approach.

This has the small side-effect that it sorts slightly different than the previously used sort function (emacs). I think this is a better sort and over all it makes it more convenient to use the script as it removes a manual step.

@bagder bagder closed this in 8f79b3e Feb 24, 2025
@bagder bagder deleted the bagder/contrithanks-shellcheck branch February 24, 2025 12:07
icing pushed a commit to icing/curl that referenced this pull request Feb 25, 2025
Now using 'sort' for sorting the names. This has the small side-effect
that it sorts slightly different than the previously used sort function
(emacs).

I think this is a better sort and over all it makes it more convenient
to use the script as it removes a manual step.

Closes curl#16448
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

4 participants