|
||
|
||
|
||
|
||
|
||
kparal commented 5 years ago | ||
jskladan commented 5 years ago Yaay, master of nitpicks! Absolutely though | ||
|
||
|
||
kparal commented 5 years ago Would it be more readable if you replaced | ||
jskladan commented 5 years ago I guess. Incidentaly, in the way I wrote the regexp, it ends up doing the same, although it was not the initial intention. My original intent was | ||
|
||
|
||
kparal commented 5 years ago Can you please explain where | ||
jskladan commented 5 years ago
The trick here (with TaskotronResultMissing type messages) is, that instead of hard-coding which arches are to be executed, you allow the fedmsg to contain an | ||
|
||
|
||
|
||
|
||
|
||
kparal commented 5 years ago I don't really understand the purpose of this code. Do you intend to add test suite coverage before merging this, or keep this comment in and do it some other time? | ||
jskladan commented 5 years ago The 'problem' here is, that the original code ( This is important when we want to be able to do I don't plan on writing a test for this before merge though, ATM. | ||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
kparal commented 5 years ago This means the missing result fedmsg can either ask for a particular arch, or otherwise all supported arches are triggered, right? That's great. | ||
jskladan commented 5 years ago Precisely | ||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
kparal commented 5 years ago Does this have any relevance to this patch? Koji has been added into pypi recently, but I don't know how well it works, have you tested it? In readme, we still require it to be installed from an rpm. | ||
jskladan commented 5 years ago Well, I have a virtualenv that does not accept external libs (because of coverage/pytest issues). Tested it with koji from pypi, and it works OK, so I decided to keep it in. Can absolutely get rid of it, if you deem it better/cleaner. /me does not care that much. | ||
kparal commented 5 years ago If it worked for you, keep it in. Just perhaps just make it a separate commit, thanks. | ||
jskladan commented 5 years ago roger | ||
|
||
|
||
|
||
Required (or at least quality of life) change for (rawhide) gating. Trigger can now consume messages with testcase
, item
, item_type
and arch
.
The fedmsg format/name/details will need to be fixed once we have the actual format, but the code stands as is.
Nitpick - add an empty line above? :-)