Hi everybody,
since a couple of hours, ansible-review is run on changes submitted through pull requests in our Ansible repository on Pagure.
This means: when someone submits a PR to the ansible repo or pushes changes into its source branch, a Zuul job is started which, when finished, reports whether or not ansible-review finds problems in changed playbooks/roles/... as a detailed comment (linking to test results) and as a flag in the side bar (see [1] as an example).
Currently this is merely informational, i.e. if the Zuul job hasn't yet finished or fails, this won't stop reviewers from merging the change into the repository regardless. On the other hand, because we've set up ansible-review to just process the changes submitted in the PR, this takes relatively little time (think about 2 minutes from push to the report coming in), so it should come in early enough unless you're in a real rush. ;)
I'd like to thank Pingou, who did the preliminary work and structure and especially Fabien Boucher who debugged the trouble we initially had with our deployed version of Zuul[2] and contributed a workaround[3,4].
Ciao, Nils
[1]: https://pagure.io/fedora-infra/ansible/pull-request/62 [2]: https://pagure.io/fedora-infra/ansible/pull-request/54 [3]: https://pagure.io/fedora-infra/ansible/pull-request/60 [4]: https://pagure.io/fedora-zuul-jobs/pull-request/60
On Thu, 14 May 2020 at 19:52, Nils Philippsen nils@redhat.com wrote:
Hi everybody,
since a couple of hours, ansible-review is run on changes submitted through pull requests in our Ansible repository on Pagure.
This means: when someone submits a PR to the ansible repo or pushes changes into its source branch, a Zuul job is started which, when finished, reports whether or not ansible-review finds problems in changed playbooks/roles/... as a detailed comment (linking to test results) and as a flag in the side bar (see [1] as an example).
Currently this is merely informational, i.e. if the Zuul job hasn't yet finished or fails, this won't stop reviewers from merging the change into the repository regardless. On the other hand, because we've set up ansible-review to just process the changes submitted in the PR, this takes relatively little time (think about 2 minutes from push to the report coming in), so it should come in early enough unless you're in a real rush. ;)
I'd like to thank Pingou, who did the preliminary work and structure and especially Fabien Boucher who debugged the trouble we initially had with our deployed version of Zuul[2] and contributed a workaround[3,4].
This is really cool, thanks to everyone involved 🎉🎉
Ciao, Nils
[4]: https://pagure.io/fedora-zuul-jobs/pull-request/60
Nils Philippsen "Those who would give up Essential Liberty to Software Engineer purchase a little Temporary Safety, deserve neither Red Hat Liberty nor Safety." -- Benjamin Franklin, 1759 PGP fingerprint: D0C1 1576 CDA6 5B6E BBAE 95B2 7D53 7FCA E9F6 395D old: C4A8 9474 5C4C ADE3 2B8F 656D 47D8 9B65 6951 3011 _______________________________________________ infrastructure mailing list -- infrastructure@lists.fedoraproject.org To unsubscribe send an email to infrastructure-leave@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/infrastructure@lists.fedorapro...
Awesome work!
So nice to have CI (of any kind) running. :)
So what does ansible-review check for currently? whats a failure?
kevin
On Thu, 2020-05-14 at 11:37 -0700, Kevin Fenzi wrote:
Awesome work!
So nice to have CI (of any kind) running. :)
So what does ansible-review check for currently? whats a failure?
I've looked into it and there isn't an easy answer, except that the way it's set up right now, we don't gate on it, its output is advisory.
As I understand the code(*), by default ansible-review considers most things it flags a mere warning, except if a variable is used bare instead of being flanked by "{{ ... }}". We could make our own standards and additional lint rules, if we needed, though.
Ciao, Nils
(*): Yeah, code, call me Luke (no, don't).
On Sat, May 16, 2020 at 02:04:57PM +0200, Nils Philippsen wrote:
On Thu, 2020-05-14 at 11:37 -0700, Kevin Fenzi wrote:
Awesome work!
So nice to have CI (of any kind) running. :)
So what does ansible-review check for currently? whats a failure?
I've looked into it and there isn't an easy answer, except that the way it's set up right now, we don't gate on it, its output is advisory.
As I understand the code(*), by default ansible-review considers most things it flags a mere warning, except if a variable is used bare instead of being flanked by "{{ ... }}". We could make our own standards and additional lint rules, if we needed, though.
I think we may want to at some point.
Does it at least catch syntax errors right now?
ie, mismatched "s or bad indents, etc?
kevin
On Sat, 2020-05-16 at 13:41 -0700, Kevin Fenzi wrote:
On Sat, May 16, 2020 at 02:04:57PM +0200, Nils Philippsen wrote:
On Thu, 2020-05-14 at 11:37 -0700, Kevin Fenzi wrote:
Awesome work!
So nice to have CI (of any kind) running. :)
So what does ansible-review check for currently? whats a failure?
I've looked into it and there isn't an easy answer, except that the way it's set up right now, we don't gate on it, its output is advisory.
As I understand the code(*), by default ansible-review considers most things it flags a mere warning, except if a variable is used bare instead of being flanked by "{{ ... }}". We could make our own standards and additional lint rules, if we needed, though.
I think we may want to at some point.
Does it at least catch syntax errors right now?
ie, mismatched "s or bad indents, etc?
Sorry it took me so long to reply. I just assumed it did, and looking into it had to find out that it doesn't.
Additionally to running ansible-review, we could let Zuul run "ansible- playbook --check ..." on affected YAML files, which would fail on problems like this.
Nils
infrastructure@lists.fedoraproject.org