Don't add reaction on comment when the command will be ignored
We give :eyes: reaction to comments that are later ignored. This may confuse user. And we do that in several situations.
- when
stgis not inpackit_instancesand/packit-stgcommand is triggered. The same forprodinstance - when some job is triggered (
/packit build,/packit propose-downstream, etc) but the corresponding job is not located in.packit.yamlfile
You can try this behavior by commenting to this PR with /packit-stg prefix.
Or to this PR by commenting with wrong job - /packit test.
This is caused because reaction to comment is given when we are processing Events but packit_instances and matching handlers are checked after that.
Way to solve this:
give reaction later (after checking packit_instances and matching handlers) so we give :eyes: reaction only when it will be followed by some other action (after all this is purpose of our bot giving a reaction)
so the goal is move part where we give a reaction in jobs.py
event.comment_object.add_reaction(COMMENT_REACTION)
somewhere "further" in code - after we check for matching handlers, job configs and after checking packit_instances
This issue has been marked as stale because it hasn't seen any activity for the last 60 days.
Stale issues are closed after 14 days, unless the label is removed by a maintainer or someone comments on it.
This is done in order to ensure that open issues are still relevant.
Thank you for your contribution! :unicorn: :rocket: :robot:
(Note: issues labeled with pinned or EPIC are never marked as stale.)
This is still valid, see https://github.com/packit/hello-world/pull/494#issuecomment-1339253487
I think I don't understand this issue. When I check your comment, Laura, Packit's actions are as expected to me:
You asked stg to process the job, there is only one set of eyes:

And the eyes are applied by the stg app

prod app did not interact with the comment in any way
all seems correct to me
The problem is staging should not react to that PR (packit_instances is not specified in the config => by default only prod app should react) and it correctly did not trigger the builds, but still reacted with eyes.
ahaaaaa, so it's about packit_instances, did not realize that; thank you for the explanation
Hi!
Would this also solve a situation, when the test job is requested whilst builds as dependencies are not finished?
The bot reacts to the comment/command (/packit test), but the test jobs won't get triggered, when build tasks finish.
For context all the test jobs are defined as manual in the config file, build jobs are set for the commit/push trigger.
As an alternative, would it work to add a reaction ("I saw it") and then an error message as a reply to the comment explaining why the command is ignored? The situations where this happens look to me like errors that should be reported somehow and not silently ignored.
An error message would be more useful than not adding a reaction, which would be a bit less confusing, but still would not really help the user with debugging. The user would be still left wondering why the reaction does not appear, like now they are wondering why nothing happens. One really needs explanatory error messages, not just an emoji. (Reminds me of the only way ed the standard text editor reported errors: a lone ?. https://news.ycombinator.com/item?id=7727206 )