Skip to content

Conversation

@hseg
Copy link

@hseg hseg commented Oct 22, 2025

(based on #2, on the presumption that that is sufficiently uncontroversial that this'll save rebase time)

In implementing #2, I found multiple corners for improvement. Not all of this is necessary, of course.

  • Basic shellcheck improvements
    Dead code elimination, quoting

  • Finish shellcheck corrections

    • Pull out common request-crafting code
    • Handle unhappy paths so the happy path is straightline code
    • Test exit codes directly to avoid them being masked
  • Use traps to clean up temp files, use mktemp
    This is less error-prone than trying to remember to do so on all exit paths. Use mktemp to avoid collisions and enable the sysadmins to configure where to put temp files.

  • Use symbolic escapes instead of codepoints
    I find them clearer

  • Use semantic markup, avoid echo
    echo's escape expansion is non-portable, using printf directly is preferable here. Or heredocs, as can be seen in some of the lengthier cases.

    See too: https://www.shellcheck.net/wiki/SC2028

  • Use explicit error codes to distinguish fail modes
    This makes it easier for users to figure out what went wrong, plus adds documentation

  • Make the prompts retry until correct input
    Also solidify them by using select loops where possible (which restricts the valid inputs)

  • Use jq more extensively for output massaging

  • Extract jq rendering code to use throughout
    These two I'm particularly proud of -- they simplify the comment selection code immensely.

  • Matters of taste

    • "Status" updates are more accurately success reports
    • Reduce color, line weight -- current choice is very noisy
    • No need to remind us at the end which reaction was picked -- it should still be on screen
    • Remove broken char in comment selection prompt

    These are absolutely understandable if you reject, and just represent my personal preferences here

Closes: #2

hseg added 12 commits October 22, 2025 15:45
Otherwise gh complains of a permissions error
Enables reacting from outside the repository
Dead code elimination, quoting
- Pull out common request-crafting code
- Handle unhappy paths so the happy path is straightline code
- Test exit codes directly to avoid them being masked
This is less error-prone than trying to remember to do so on all exit
paths. Use mktemp to avoid collisions and enable the sysadmins to
configure where to put temp files.
This is clearer to the reader
echo's escape expansion is non-portable, using printf directly is
preferable here. Or heredocs, as can be seen in some of the lengthier
cases.

See too: https://www.shellcheck.net/wiki/SC2028
Also solidify them by using select loops where possible (which restricts
the valid inputs)
- "Status" updates are more accurately success reports
- Reduce color, line weight -- current choice is very noisy
- No need to remind us at the end which reaction was picked -- it should
  still be on screen
- Remove broken char in comment selection prompt
@hseg hseg force-pushed the style-improvements-split branch from ef10509 to 8cabc22 Compare October 22, 2025 20:07
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.

1 participant