Wonderful! Thank you for this contribution, Sandor!
On Tue, Feb 19, 2019 at 10:14 AM Sandeep Moré <moresand...@gmail.com> wrote: > 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 > > >> > > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > > > > >
