"We can't do this unless we only accept PRs. It is entirely possible to commit to the repository without opening a PR so we want tests to run. The Knox-master-daily job also runs full integration release tests and not just the unit tests that are run on PRs currently."
Got it; thanks for the clarification! On Fri, Feb 8, 2019 at 3:22 PM Kevin Risden <[email protected]> wrote: > I created https://issues.apache.org/jira/browse/KNOX-1759 with some > subtasks for concrete action items to take to move forward. > > "If that happened we might get rid of the > 'Knox-master-daily' job since it being executed after a commit is merged > into master (AFAIK) which makes no sense if we only allow a commit to be > merged if all tests were successfully passed already" > > We can't do this unless we only accept PRs. It is entirely possible to > commit to the repository without opening a PR so we want tests to run. The > Knox-master-daily job also runs full integration release tests and not just > the unit tests that are run on PRs currently. > Kevin Risden > > > On Fri, Feb 8, 2019 at 4:34 AM Sandor Molnar <[email protected] > > > wrote: > > > +1 for PRs. > > > > My two cents on Kevin's list: > > - PR template is a good idea; Ambari also has one here: > > > > > https://github.com/apache/ambari/blob/trunk/.github/PULL_REQUEST_TEMPLATE.md > > It would also be great if test steps are described in a detailed manner > > (it helped me many times in case I had to reproduce something months > after > > the PR was merged) > > > > - comments on the PR: in case of Ambari they go to the 'Worklog' tab in > the > > corresponding JIRA, which - IMO - was better than put all of these stuff > > within the comments; it gave us a clear separation and did not spam the > > comments in the JIRA where other useful information may be found (i.e. a > > design history, open point clarification, etc...). Not to mention that > the > > worklogs contain many information > > > > - link the PRs to the JIRA automatically is essential IMO; thanks for > > pointing that out Kevin! > > > > - I'm not sure if it is feasible (currently does not seem to be the case) > > but it would be great if contributors could invite others for review > (i.e. > > not only committers) > > > > - Apache has a Jenkins instance to run CI checks on its projects (Ambari > > sample: https://builds.apache.org/job/Ambari-Github-PullRequest-Builder/ > ). > > Since Knox already has some jobs here (for instance > > https://builds.apache.org/job/Knox-master-daily/) we might consider > > creating Knox's PR Builder here too (so that all of our CI related jobs > > would be in one place). If that happened we might get rid of the > > 'Knox-master-daily' job since it being executed after a commit is merged > > into master (AFAIK) which makes no sense if we only allow a commit to be > > merged if all tests were successfully passed already > > > > Cheers, > > Sandor > > > > On Fri, Feb 8, 2019 at 5:53 AM Jeffrey Rodriguez <[email protected]> > > wrote: > > > > > +1 It is great that we are considering Pull request that would help to > > > increase community collaboration. > > > Jeffrey E Rodriguez > > > > > > On Thu, Feb 7, 2019 at 3:43 PM Robert Levas > <[email protected] > > > > > > wrote: > > > > > > > +1. I think this is a great idea. > > > > > > > > On Thu, Feb 7, 2019 at 5:29 PM larry mccay <[email protected]> > wrote: > > > > > > > > > Great list of ideas/practices there, Kevin! > > > > > > > > > > I for one would want comments added as comments to JIRA. > > > > > I hate coming across a JIRA that would address something that I am > > > > looking > > > > > for and then find no meaningful comments. > > > > > > > > > > > > > > > On Thu, Feb 7, 2019 at 4:20 PM Phil Zampino <[email protected]> > > > wrote: > > > > > > > > > > > +1, let's follow good models from the community, and save > ourselves > > > > those > > > > > > headaches which can be avoided. > > > > > > > > > > > > On Thu, Feb 7, 2019 at 4:17 PM > > Kevin Risden > > <[email protected]> > > > > wrote: > > > > > > > > > > > > > I think PRs are a good improvement since we get Travis CI > checks > > by > > > > > > default > > > > > > > currently. Something that we currently don't get with patches > > > > > > > > > > > > > > If we go this route we should make sure we have the following > in > > > > place: > > > > > > > > > > > > > > - PR Github Template with useful info > > > > > > > - > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://help.github.com/articles/creating-a-pull-request-template-for-your-repository/ > > > > > > > - Livy has an example of this > > > > > > > - Ensure that PRs are automatically linked to JIRA > > > > > > > - Not currently done today and a pain since it should > > happen > > > > > > > automatically. > > > > > > > - Calcite has this. Might be a simple INFRA ticket > > > > > > > - Documentation for contributors/committers > > > > > > > - Committers - linked github/asf accounts, how to merge a > > PR > > > > > > > - Contributors what to expect/updated docs to move from > > patch > > > > -> > > > > > PR > > > > > > > - Ensure that only squash/rebase/merge commits are allowed > > > > > > > - Lot of nuance here and Calcite recently had INFRA > disable > > > the > > > > > > > buttons for types that didn't fit their model > > > > > > > - Decided what to do with PR comments > > > > > > > - Some projects have PR comments go directly to JIRA > > > comments. > > > > > > > - Others have them go to worklog in JIRA. > > > > > > > - Others don't capture PR comments in JIRA > > > > > > > > > > > > > > So all in all in favor just need to make sure we have the > > plumbing > > > in > > > > > > > place. > > > > > > > Kevin Risden > > > > > > > > > > > > > > On Thu, Feb 7, 2019 at 4:13 PM Sandeep Moré < > > [email protected] > > > > > > > > > > wrote: > > > > > > > > > > > > > > > I second Phil. Personally, I am more comfortable with the > > patches > > > > > > mostly > > > > > > > > because of their simplistic nature but do like PRs as they > are > > > more > > > > > > > > community friendly (helps people review, comment, critique) > and > > > > looks > > > > > > > like > > > > > > > > they have become OSS standard as Phil pointed out. > > > > > > > > So +1 from me. > > > > > > > > > > > > > > > > Best, > > > > > > > > Sandeep > > > > > > > > > > > > > > > > On Thu, Feb 7, 2019 at 4:06 PM Phil Zampino < > > [email protected] > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > I like the PR model, and it is familiar to many who > > contribute > > > to > > > > > OSS > > > > > > > > > projects. I suppose we could continue to support the patch > > > > attached > > > > > > to > > > > > > > a > > > > > > > > > Jira model, but we should encourage the PR model, IMHO. > > > > > > > > > > > > > > > > > > On Thu, Feb 7, 2019 at 4:03 PM larry mccay < > > [email protected]> > > > > > > wrote: > > > > > > > > > > > > > > > > > > > All - > > > > > > > > > > > > > > > > > > > > There has been interest from the Knox community in > support > > of > > > > > Pull > > > > > > > > > Requests > > > > > > > > > > from github. > > > > > > > > > > Our move to gitbox recently makes this easier to do. > > > > > > > > > > > > > > > > > > > > What are your thoughts on enabling PRs in general? > > > > > > > > > > Should we support both patches in JIRA as well as github > > > based > > > > > PRs? > > > > > > > > > > > > > > > > > > > > thanks, > > > > > > > > > > > > > > > > > > > > --larry > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
