Skip to content

Conversation

@mhucka
Copy link
Contributor

@mhucka mhucka commented Mar 4, 2025

This is a rewritten version of check/format-incremental. This was motivated by the desire to add a --help option and do a little bit of cleaning up the code, and then one thing led to another, and, well, here we are. The final changes include:

  • Added a --help option
  • Added a --no-color option
  • Added a --quiet option
  • Make the script more robust in various ways (e.g., declare variables, be more careful in parsing CLI arguments, etc.)
  • Don't hard-code ANSI color code sequences in every echo statement
  • Make the script more DRY
  • Revise the error & info messages to try to be slightly more clear

mhucka added 2 commits March 4, 2025 08:49
This is a rewritten version of `check/format-incremental`. This was
motivated by the desire to add a `--help` option and do a little bit
of cleaning up the code, and then one thing led to another, and, well,
here we are. The final changes include:

- Added a `--help` option
- Added a `--no-color` option
- Added a `--quiet` option
- Make the script more robust in various ways (e.g., declare
  variables, be more careful in parsing CLI arguments, etc.)
- Don't hard-code ANSI color code sequences in every echo statement
- Make the script more DRY
- Revise the error & info messages to try to be slightly more clear
@mhucka mhucka marked this pull request as ready for review March 4, 2025 17:20
@mhucka mhucka requested review from mpharrigan and ncrubin March 4, 2025 18:41
@mhucka mhucka self-assigned this Mar 4, 2025
@mpharrigan
Copy link
Collaborator

Is this shared with the other various repos? Is this the first place the updated script will be committed?

@mpharrigan
Copy link
Collaborator

my personal rule is once you start using a for loop in a bash script, it's probably time to port it to something like python. Is there someone with more bash expertise who can review this?

@mhucka
Copy link
Contributor Author

mhucka commented Mar 7, 2025

Is this shared with the other various repos?

Not yet, but that's my intention. I had to add one for the unitary repo, took the opportunity to rewrite it, and now am going around to add it to the other repos.

Is this the first place the updated script will be committed?

It's in a PR for here and unitary at the momment.

my personal rule is once you start using a for loop in a bash script, it's probably time to port it to something like python. Is there someone with more bash expertise who can review this?

I think what I hear you saying is that it's become too complicated. OK, I worked on simplifying it just now and will push an update.

@pavoljuhas is more expert in Bash than me, so he's the only other one I can think of. I'll add him to the reviewers.

@mhucka
Copy link
Contributor Author

mhucka commented Mar 7, 2025

@mpharrigan The latest commit has a simplified version of the code.

@mhucka mhucka requested a review from pavoljuhas March 7, 2025 22:02
@mhucka mhucka changed the title Improve format-incremental scripts Improve format-incremental script Mar 13, 2025
@mpharrigan
Copy link
Collaborator

Thanks @mhucka ! It would be nice if @pavoljuhas could give a thorough review so we can port this to all the repos with confidence

@mhucka mhucka marked this pull request as draft March 31, 2025 21:44
@mhucka mhucka closed this Apr 24, 2025
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