<div dir="ltr"><div>Overall if for a tool we don't care if it down and consider it's output to be something to ignore it would seem to offer little value, I would suggest we aim for it to be available with useful output.</div><div><br></div><div>Will this just be adding a comment or will this also be influencing the status of the pull request - i.e. will these reports be affecting the tick or the cross visible next to a PR? When looking at the PR queue the ones with the green ticks are the easiest to jump into, provided no changes are required to the code they are potentially ready to merge - adding false crosses would affect this.</div><div dir="ltr"><br></div><div dir="ltr">On Tue, Apr 9, 2019 at 6:47 AM Martin Stefanko <<a href="mailto:mstefank@redhat.com">mstefank@redhat.com</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">> <span style="font-family:"trebuchet ms",sans-serif">Does the commit check run against all commits in the PR?</span><div><font face="trebuchet ms, sans-serif"><br></font></div><div><font face="trebuchet ms, sans-serif">Not currently, I've added additional checks to verify there is only one commit in the PR :) But this should be possible to do, however, force pushing your way through commit history will not be an easy task.</font></div></div></blockquote><div><br></div><div>I think instead it should be checking all commits, it often makes sense to break work into logical chunks - often it is easier to review commit by commit.</div><div><br></div><div>As an example it is a bad practice to bump a subsystem version and add to it within a single commit, GitHub provides no way to diff files like schemas when that happens so the only way to check is to pull the PR locally and manually diff. I suspect the manual diff is a very rare event if it happens at all. Component upgrades whilst could be in a single commit often make sense to be in individual commits. When working on an issue and encountering a related but different bug it makes sense to use a separate Jira issue and commit, it is not always practical to submit this fix in it's own PR. If working on a piece of work and another engineer also follows on you need to ensure you don't modify any shared commits etc... </div><div><br></div><div>Overall the human code review is still mandatory and part of that review would consider the commits and if they make sense.</div><div><br></div><div>I think something that could be useful could be to list the checks to be applied somewhere to make it easier to discuss individually, as this would be a change to the WildFly Core and WildFly pull requests it could even make sense to submit as a proposal to the wildfly-proposals repo.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><font face="trebuchet ms, sans-serif"><br></font></div><div><font face="trebuchet ms, sans-serif">> </font><span style="font-family:"trebuchet ms",sans-serif">What happens if it is down? I figure nothing really, i.e. the check just isn't performed, which is a quite reasonable failure mode. :) </span></div><div><span style="font-family:"trebuchet ms",sans-serif"><br></span></div><div><span style="font-family:"trebuchet ms",sans-serif">> </span><span style="font-family:"trebuchet ms",sans-serif">AIUI this also has no impact on ability to merge PRs; i.e. we can ignore the result same as we can ignore CI job results. Also good.</span></div><div><font face="trebuchet ms, sans-serif"><br></font></div><div><font face="trebuchet ms, sans-serif">Absolutely right. There will be only one additional status in the list. Everything can be ignored.</font></div></div></blockquote><div><br></div><div>Absolutely everything we add to the status list we should be aiming to be accurate, CI is complex and a never ending task to keep on top of intermittent issues and flaky tests but the objective is still to reach that point. We should not be adding something else to this list deliberately with false negative reviews.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><font face="trebuchet ms, sans-serif"><br></font></div><div><font face="trebuchet ms, sans-serif">> </font>We can run it in CI somewhere if we need to, I'll need to review the docs and see whats required to run it.</div><div><br></div><div>Ken, I will take this with you offline when it's decided that we can set it up. <br clear="all"><div><div dir="ltr" class="gmail-m_3799414308558704619gmail_signature"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div><br></div><div>Martin <br></div></div></div></div></div></div></div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Apr 8, 2019 at 4:15 PM Ken Wills <<a href="mailto:kwills@redhat.com" target="_blank">kwills@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Apr 8, 2019 at 7:32 AM Martin Stefanko <<a href="mailto:mstefank@redhat.com" target="_blank">mstefank@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Yes, understandable. Maybe it can be run where pull-player is being run then? CC Ken.<br clear="all"></div></blockquote><div><br></div><div>We can run it in CI somewhere if we need to, I'll need to review the docs and see whats required to run it.</div><div><br></div><div>For the initial version, I think we should just try it in OS though, then we can move it at some point in the future.</div><div><br></div><div>Ken</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><div dir="ltr" class="gmail-m_3799414308558704619gmail-m_8603178563120312178gmail-m_1565700362276790637gmail_signature"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div><br></div><div>Martin <br></div></div></div></div></div></div></div><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Apr 8, 2019 at 12:27 PM Darran Lofthouse <<a href="mailto:darran.lofthouse@jboss.com" target="_blank">darran.lofthouse@jboss.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">I think if we were to add something to the PR process is should not rely on a single individual to maintain it, if that person is busy, asleep, sick or on holiday then there will be no one to deal with down time and other issues,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Apr 8, 2019 at 11:25 AM Martin Stefanko <<a href="mailto:mstefank@redhat.com" target="_blank">mstefank@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">I can run it my openshift online instance so uptime would be maintained by me for the time being. I have already several instances running like this for some time. During F2F we've also discussed possibility to use <a href="https://github.com/kubernetes/test-infra/tree/master/prow" target="_blank">Prow project</a> for wildlfy which would require a platform to run (my openshift cannot cover this) and Tyr would nicely integrate with Prow (already tested). However, I am not aware if there has been any movement with Prow setup for wildfly repository recently.<br clear="all"><div><div dir="ltr" class="gmail-m_3799414308558704619gmail-m_8603178563120312178gmail-m_1565700362276790637gmail-m_-9073834282964421639gmail-m_-6589929154998680002gmail_signature"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div><br></div><div>Martin<br></div></div></div></div></div></div></div><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Apr 8, 2019 at 11:26 AM Darran Lofthouse <<a href="mailto:darran.lofthouse@jboss.com" target="_blank">darran.lofthouse@jboss.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Couple of questions.<div><br></div><div>Where does this need to run?</div><div><br></div><div>How does this get maintained regarding things like uptime?</div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Apr 8, 2019 at 9:52 AM Martin Stefanko <<a href="mailto:mstefank@redhat.com" target="_blank">mstefank@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr">Hi,</div><div dir="ltr"><br></div><div dir="ltr">I would like to present my side project Tyr [1] which is a tool that can verify GitHub pull request structure by a YAML definition (example in [2]) provided by a user. The PR author sees the violations immediately in the PR status which allows to correct mistakes right away when the PR is created.</div><div dir="ltr"><br></div><div dir="ltr">I've recorded a short demo available at [3].</div><div dir="ltr"><br></div><div>The validation is fully configurable and can be extended with for instance analysis document links and similar.</div><div dir="ltr"><br></div><div dir="ltr">I would like to include this functionality in wildfly repository. Thoughts?</div><div dir="ltr"><div><br></div><div>[1] <a href="https://github.com/xstefank/tyr" target="_blank">https://github.com/xstefank/tyr</a></div><div>[2] <a href="https://gist.github.com/xstefank/3a79f2f199ee1e449a44c607120a9d30" target="_blank">https://gist.github.com/xstefank/3a79f2f199ee1e449a44c607120a9d30</a></div><div>[3] <a href="https://youtu.be/qZRcMQ6qIpg" target="_blank">https://youtu.be/qZRcMQ6qIpg</a><br clear="all"><div><div dir="ltr" class="gmail-m_3799414308558704619gmail-m_8603178563120312178gmail-m_1565700362276790637gmail-m_-9073834282964421639gmail-m_-6589929154998680002gmail-m_-7932499814739059909gmail-m_-6243839654698323803gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div><br></div><div>Martin Stefanko</div><div><br></div><div>Software Engineer</div><div>Middleware Runtimes Sustaining Engineering Team</div><div>Red Hat</div></div></div></div></div></div></div></div></div></div></div></div></div></div></div>
_______________________________________________<br>
wildfly-dev mailing list<br>
<a href="mailto:wildfly-dev@lists.jboss.org" target="_blank">wildfly-dev@lists.jboss.org</a><br>
<a href="https://lists.jboss.org/mailman/listinfo/wildfly-dev" rel="noreferrer" target="_blank">https://lists.jboss.org/mailman/listinfo/wildfly-dev</a></blockquote></div>
</blockquote></div>
</blockquote></div>
</blockquote></div>
</blockquote></div></div>
</blockquote></div>
_______________________________________________<br>
wildfly-dev mailing list<br>
<a href="mailto:wildfly-dev@lists.jboss.org" target="_blank">wildfly-dev@lists.jboss.org</a><br>
<a href="https://lists.jboss.org/mailman/listinfo/wildfly-dev" rel="noreferrer" target="_blank">https://lists.jboss.org/mailman/listinfo/wildfly-dev</a></blockquote></div></div>