The commits will probably be easier to review if taken one by one :)
Screenshot, because they are always nice :)
Looks cool to me, though it might be nice to have some tests to assert all the if statements in the new template code.
Also, Pagure is 100% the right place to report CI results instead of Bodhi!
Awesome addition!
I think it would be preferable to use split_message here, or maybe message_parts
split_message
message_parts
Add some whitespace around the | operator? {{ commitid | short }}
|
{{ commitid | short }}
Whitespace around operator |, and maybe parentheses to clarify intent regarding operator precedence of | vs ==
==
in other places, there are spaces between tag content: {{ commit.parents[0].oid.hex }}
{{ commit.parents[0].oid.hex }}
whitespace around |, maybe parentheses again
maybe extra whitespace {{ parent.oid.hex }}
{{ parent.oid.hex }}
missing whitespace for consistency {% endif %}
{% endif %}
consistent whitespace around |, maybe parentheses
several whitespace inconsistencies in the following section, I didn't mark them all they can obfuscate intent, e.g. {{'s' if diff|count > 1 }} could be {{ 's' if diff | (count > 1) }}
{{'s' if diff|count > 1 }}
{{ 's' if diff | (count > 1) }}
extra whitespace
Do we really want to continue the line here? If this is usually done in Pagure then ok, but I feel like this is something to avoid. You could just concatenate the strings.
1 new commit added
4 new commits added
Looks good, and more consistent now. Thanks!
Thanks for the review! :)
Pull-Request has been merged by pingou