Great, thanks Sandor ! On Tue, Feb 19, 2019 at 10:09 AM Sandor Molnar <smol...@cloudera.com.invalid> wrote:
> Hi folks! > > It's all set; you can check out the umbrella JIRA for further information: > https://issues.apache.org/jira/browse/KNOX-1759 > > Additionally, you might want to read the enhanced documentation on how to > contribute using GitHub PRs here: > > https://cwiki.apache.org/confluence/display/KNOX/Contribution+Process#ContributionProcess-GithubWorkflow > > Regards, > Sandor > > On Fri, Feb 8, 2019 at 4:10 PM Sandor Molnar <smol...@cloudera.com> wrote: > > > "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 <kris...@apache.org> 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 > <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 > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > > >
