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 <smol...@cloudera.com.invalid> 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 <jeffrey...@gmail.com> > 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 <rle...@cloudera.com.invalid > > > > wrote: > > > > > +1. I think this is a great idea. > > > > > > On Thu, Feb 7, 2019 at 5:29 PM larry mccay <lmc...@apache.org> 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 <pzamp...@gmail.com> > > 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 > <kris...@apache.org> > > > 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é < > moresand...@gmail.com > > > > > > > > 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 < > pzamp...@apache.org > > > > > > > > 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 < > lmc...@apache.org> > > > > > 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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
