Update the epilog of the help comment#3124
Conversation
There are multiple supported file names of Packi config files. The help message has been edited to reflect this fact.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the help comment constants and the logic for generating help messages in worker handlers, introducing modular constants and conditional notes for Pagure projects. The review feedback recommends improving the visual formatting of help messages with additional newlines, using f-strings for string concatenation, and adding a safety check for null comment values to prevent potential runtime errors.
| " - *Mastodon*: @packit@fosstodon.org\n" | ||
| ) | ||
| HELP_COMMENT_DOCS = "**Documentation**: \n - {docs_url}" | ||
| HELP_COMMENT_EPILOG = "{note}" + HELP_COMMENT_CONTACT + HELP_COMMENT_DOCS |
There was a problem hiding this comment.
For better readability and consistency with modern Python practices, consider using an f-string for concatenation and formatting.
| HELP_COMMENT_EPILOG = "{note}" + HELP_COMMENT_CONTACT + HELP_COMMENT_DOCS | |
| HELP_COMMENT_EPILOG = f"{note}{HELP_COMMENT_CONTACT}{HELP_COMMENT_DOCS}" |
There was a problem hiding this comment.
This wouldn't work as note would be undefined in this context
|
Build succeeded. ✔️ pre-commit SUCCESS in 2m 05s |
With the previous implementation, when a user would use the `/packit help` command, the epilog would mention the Fedora CI `/packit-cit help` command, which would confuse users who do not know what Fedora CI even is. The note is now only added when the `/packit help` command is used in the context of Pagure.
The implementation is now more readable. Assisted-By: Claude Sonnet 4.5 noreply@anthropic.com
betulependule
force-pushed
the
help-comment-epilog-update
branch
from
d26a691 to
6a319a2
Compare
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 56s |
This should never happen in the current implementation, but add this prevention in case this error is ever introduced due to changes in the codebase.
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 50s |
4 participants
This PR introduces some small edits to the epilog of the help comment.
.packit.yamland instead, uses a more generic term./packit-ciis now only added when the user uses/packit helpon Pagure. Many users of Packit don't know what Fedora CI even is, so mentioning it on GitHub and Gitlab made little sense.I've tested the fixes on a local deployment, but am not currently able to test it after applying the latest refactoring changes as I am hitting a
Temporary failure in name resolution. I'll re-test it once I am able to.TODO:
Not sure if this is relevant enough to be mentioned in the blog post. If yes, here are the release notes below. Ignore this otherwise:
RELEASE NOTES BEGIN
In instances where the help comment (created in response to
/packit help) would reference the need for a configuration file, Packit now uses a generic term, reflecting support for all valid configuration file names and replacing the previous specific mention of .packit.yaml. The help comment also no longer references/packit-cicommands on GitHub and Gitlab as these commands are not relevant to users on these platforms.RELEASE NOTES END