Skip to content

maint ~ configuration and utility clean-up and maintenance #3090

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

Merged
merged 18 commits into from
Feb 13, 2022

Conversation

rivy
Copy link
Member

@rivy rivy commented Feb 6, 2022

  • waiting for the CI to pass to make sure I didn't make any silly mistakes.

@rivy rivy marked this pull request as draft February 7, 2022 14:55
@@ -77,14 +77,14 @@ jobs:
run: |
## Build binaries
cd '${{ steps.vars.outputs.path_UUTILS }}'
bash util/build-gnu.sh
bash util/build-gnu.bash
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this change?
why not making them .sh compatible instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I just jumped to the rename for shellcheck.

But I think it's just a variable expansion, so it should be reasonably simple to change.
I'll look at it from that angle and try to rework it as POSIX-shell compatible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, dropping this rename (as unneeded) in the revision.
I must have been using an old shellcheck on a different PC; checking now shows no complaints.

@rivy
Copy link
Member Author

rivy commented Feb 7, 2022

I modified something incorrectly in the scripts supporting 'GnuTests'. 😕
When I get that sorted, I'll change back to PR status.

@sylvestre
Copy link
Contributor

I guess it is normal that it is still a draft :)

@rivy rivy force-pushed the maint.utils branch 2 times, most recently from c92b1fd to bf192a8 Compare February 12, 2022 15:47
@rivy
Copy link
Member Author

rivy commented Feb 12, 2022

@sylvestre , well, that was a slog trying to iterate changes and debug a process which takes 1 to 2 hours for feedback. 😅

I plan to change this to a PR in a couple of hours (when CI passes).

@rivy
Copy link
Member Author

rivy commented Feb 12, 2022

@sylvestre , @tertsdiepraam , this is ready for comments/review.

I just tweaked some commit comments which triggered another CI run, but CI is passing.

@rivy rivy marked this pull request as ready for review February 12, 2022 16:58
@rivy rivy requested a review from sylvestre February 12, 2022 16:58
do
sum_path="${BUILDDIR}/${sum}"
test -f "${sum_path}" || cp "${BUILDDIR}/hashsum" "${sum_path}"
for sum in b2sum md5sum sha1sum sha224sum sha256sum sha384sum sha512sum; do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for sum in b2sum md5sum sha1sum sha224sum sha256sum sha384sum sha512sum; do
for sum in b2sum b3sum md5sum sha1sum sha224sum sha256sum sha384sum sha512sum; do

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻

@rivy rivy force-pushed the maint.utils branch 2 times, most recently from 41b6321 to d76a60e Compare February 12, 2022 18:46
@rivy
Copy link
Member Author

rivy commented Feb 12, 2022

Added a commit fixing a cargo clippy complaint for head which slipped into the 'main' tree with a recent commit.
I'll refrain from further rebasing onto 'main'.

Any other comments/concerns?

@sylvestre
Copy link
Contributor

sounds great :)

@sylvestre
Copy link
Contributor

and it was fixed by the initial author: #3124

@rivy
Copy link
Member Author

rivy commented Feb 12, 2022

and it was fixed by the initial author: #3124

I'll drop the fix commit and rebase off the new 'main' then...

@rivy
Copy link
Member Author

rivy commented Feb 12, 2022

Done... rebased and pushed onto the fixed 'main'.
That should pass (🤞🏻) and then be merge-able if you concur.

@sylvestre sylvestre merged commit 78e2fe6 into uutils:main Feb 13, 2022
@sylvestre
Copy link
Contributor

excellent, thanks

@sylvestre
Copy link
Contributor

More conflicts on #3045 ;)

@rivy
Copy link
Member Author

rivy commented Feb 13, 2022

I'm on it.

@rivy rivy deleted the maint.utils branch February 13, 2022 16:43
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

Successfully merging this pull request may close these issues.

2 participants