FYI... check out https://issues.apache.org/jira/browse/INFRA-16602 where a lot of this conversation is taking place.
On Wed, Aug 1, 2018 at 7:16 AM Taylor Edmiston <tedmis...@gmail.com> wrote: > To Fokko's point: > > > 2. We need to make sure that we close the Jira ourself. > > Do we have a preference between the issue being closed by the PR sender vs > the PR merger? I had one merged today but didn't want to disrupt the > process if someone is already working on getting Jira issue closing > automated again, etc. > > Taylor > > On Wed, Aug 1, 2018 at 5:14 AM, Sergei Iakhnin <lle...@gmail.com> wrote: > > > Thanks for addressing this quickly Ash! > > > > On Wed, Aug 1, 2018 at 11:06 AM Ash Berlin-Taylor < > > ash_airflowl...@firemirror.com> wrote: > > > > > > > > > On 1 Aug 2018, at 09:43, Ash Berlin-Taylor < > > > ash_airflowl...@firemirror.com> wrote: > > > > > > > > Hi Sergi, > > > > > > > > Yes, I agree, and I've asked (as a short term measure) for the > comments > > > to list and comments on every PR duplicating to Jira to be removed (so > > that > > > the volume was as it was) -- we can then think about what we want to > > > re-enable, perhaps to a different list. > > > > > > > > > > I've opened https://issues.apache.org/jira/browse/INFRA-16854 to ask > > them > > > to remove the comment integration so the volume of mails should drop > once > > > they get around to it. I don't know how long that will take. > > > > > > Sorry about the noise - I didn't know Gitbox would integrate comments > > like > > > this as nothing I read about the process mentioned this. > > > > > > -ash > > > > > > > Fokko: the new Apache-hosted URL is > > > https://gitbox.apache.org/repos/asf?p=incubator-airflow.git > > > > > > > > We should ask if they can remove/hide the old non-gitbox one. > > > > > > > > -ash > > > > > > > >> On 1 Aug 2018, at 08:35, Sergei Iakhnin <lle...@gmail.com> wrote: > > > >> > > > >> I find the dev list has gotten extremely noisy with the move to > > Github. > > > >> Getting an email about every PR comment seems a bit excessive. Might > > it > > > be > > > >> a good idea to not have this list subscribed to all the github > > updates? > > > >> People who are interested in such granular updates can still "watch" > > the > > > >> github repo to opt in. > > > >> > > > >> With best regards, > > > >> > > > >> Sergei. > > > >> > > > >> > > > >> On Wed, Aug 1, 2018 at 9:30 AM Driesprong, Fokko > <fo...@driesprong.frl > > > > > > >> wrote: > > > >> > > > >>> Hi Max, > > > >>> > > > >>> We're totally on the same page, I think I've phrased it a bit > clumsy. > > > >>> > > > >>> Two things that I've noticed: > > > >>> > > > >>> 1. Apache is not being mirrored, is this expected behaviour? > > > >>> > > > >>> MacBook-Pro-van-Fokko:incubator-airflow fokkodriesprong$ git reset > > > --hard > > > >>> apache/master > > > >>> > > > >>> HEAD is now at dfa7b26d [AIRFLOW-2809] Fix security issue regarding > > > Flask > > > >>> SECRET_KEY > > > >>> > > > >>> MacBook-Pro-van-Fokko:incubator-airflow fokkodriesprong$ git reset > > > --hard > > > >>> github/master > > > >>> > > > >>> HEAD is now at ed972042 [AIRFLOW-1104] Update jobs.py so Airflow > does > > > not > > > >>> over schedule tasks (#3568) > > > >>> > > > >>> 2. We need to make sure that we close the Jira ourself. > > > >>> > > > >>> Cheers, Fokko > > > >>> > > > >>> > > > >>> > > > >>> > > > >>> 2018-07-31 21:50 GMT+02:00 Maxime Beauchemin < > > > maximebeauche...@gmail.com>: > > > >>> > > > >>>> What I meant by changing history is mutating one or many SHAs in > the > > > >>>> branch, an operation that would require force-pushing, which > merging > > > >>>> doesn't do. Personally I prefer "Squash & Merge" as it makes for a > > > >>>> merge-commit free `git log` and having a linear branch history in > > > master > > > >>>> that aligns with when things were introduced to the branch. > > > >>>> > > > >>>> It's possible to disable some of these options from the repo (only > > if > > > >>>> you're an Admin, meaning we'd have to involve INFRA to change > that). > > > But > > > >>>> it's good to have options for the cases I mentioned above. > > > >>>> > > > >>>> So committers, use "Squash and Merge"! It matches our previous > > process > > > >>> when > > > >>>> using the defaults in the now defunct `scripts/airflow-pr` > > > >>>> > > > >>>> [I'm really hoping I'm not starting a merge vs rebase workflow > > debate > > > >>>> here...] > > > >>>> > > > >>>> Max > > > >>>> > > > >>>> On Tue, Jul 31, 2018 at 12:37 PM Driesprong, Fokko > > > <fo...@driesprong.frl > > > >>>> > > > >>>> wrote: > > > >>>> > > > >>>>> Hi Max, > > > >>>>> > > > >>>>> You're right. I just started plowing though my mailbox and > merged a > > > >>>> commit > > > >>>>> without squash and merge, but it changes history as you mention. > > > >>>>> Nice thing of Github is if you change it, it remembers your > > > preference > > > >>>>> which is Squash and Merge :-) > > > >>>>> > > > >>>>> Love the Gitbox so far, great work! > > > >>>>> > > > >>>>> Cheers, Fokko > > > >>>>> > > > >>>>> 2018-07-31 21:34 GMT+02:00 Maxime Beauchemin < > > > >>> maximebeauche...@gmail.com > > > >>>>> : > > > >>>>> > > > >>>>>> "Squash & Merge" (the default) does the right thing (squashes > the > > > >>>>> multiple > > > >>>>>> commit and replays the resulting commit on top of master), we > > should > > > >>>> use > > > >>>>>> that most of the times. We'd only want to merge if we wanted to > > > >>>> preserve > > > >>>>>> history from within the PR (multiple collaborators or multiple > > > >>>> important > > > >>>>>> commits that we want to keep detailed in master for instance). > > > >>>>>> > > > >>>>>> I'm not sure how to verify whether the `master` branch is > > protected > > > >>> on > > > >>>>> this > > > >>>>>> setup (without pushing to it as a test, which I'd rather not > do). > > We > > > >>>>> should > > > >>>>>> make sure that it is though as changing history on master can > > cause > > > >>> all > > > >>>>>> sorts of problems. > > > >>>>>> > > > >>>>>> Max > > > >>>>>> > > > >>>>>> On Tue, Jul 31, 2018 at 9:21 AM Sid Anand <san...@apache.org> > > > wrote: > > > >>>>>> > > > >>>>>>> The other benefit of using Option 3 over Option 1 is that you > > > >>>> maintain > > > >>>>>> the > > > >>>>>>> history of who committed and who authored in one line in the > Git > > > >>>> log-- > > > >>>>>> i.e. > > > >>>>>>> "bob33 authored and ashb committed 3 hours ago" instead of just > > > >>> "ashb > > > >>>>>>> committed" for a merge commit followed by the commit(s) from > > bob33. > > > >>>>>>> > > > >>>>>>> On Tue, Jul 31, 2018 at 9:11 AM Sid Anand <san...@apache.org> > > > >>> wrote: > > > >>>>>>> > > > >>>>>>>> Ash, > > > >>>>>>>> This is pretty cool. I just merged one PR from GH directly. > > > >>>>>>>> > > > >>>>>>>> Interestingly, I still used the `dev/airflow-pr work_local` to > > > >>> test > > > >>>>> out > > > >>>>>>>> the PR, but merging directly in the GitHub UI afterwards > > > >>> definitely > > > >>>>>>> avoided > > > >>>>>>>> my needing to do another `dev/airflow-pr merge` CLI command. > > > >>>>>>>> > > > >>>>>>>> There are 3 options in the UI: The default is "Create a merge > > > >>>> commit" > > > >>>>>>>> (Option 1). I think the ones we want is the "Rebase & Merge" > > > >>>> (Option > > > >>>>>> 3), > > > >>>>>>>> which requires that PR submitters squash their commits. > > > >>> Otherwise, > > > >>>> we > > > >>>>>>> could > > > >>>>>>>> use "Squash & Merge" (Option 2), though I am not clear if > Squash > > > >>> & > > > >>>>>> Merge > > > >>>>>>> is > > > >>>>>>>> more like option 1 or option 3. > > > >>>>>>>> > > > >>>>>>>> -s > > > >>>>>>>> > > > >>>>>>>> On Mon, Jul 30, 2018 at 7:19 PM Andrew Phillips < > > > >>>>> aphill...@qrmedia.com > > > >>>>>>> > > > >>>>>>>> wrote: > > > >>>>>>>> > > > >>>>>>>>>> We should ask Apache infra to send the GH notifs to another > > > >>>>> mailing > > > >>>>>>>>>> list. > > > >>>>>>>>> > > > >>>>>>>>> Over at jclouds, we created a "notifications@" list for this > > > >>>>> purpose > > > >>>>>>>>> (well, actually we renamed "issues@" to "notifications@"), > and > > > >>>> send > > > >>>>>>>>> messages there: > > > >>>>>>>>> > > > >>>>>>>>> https://issues.apache.org/jira/browse/INFRA-7180 > > > >>>>>>>>> > > > >>> https://mail-archives.apache.org/mod_mbox/jclouds-notifications/ > > > >>>>>>>>> > > > >>>>>>>>> Regards > > > >>>>>>>>> > > > >>>>>>>>> ap > > > >>>>>>>>> > > > >>>>>>>> > > > >>>>>>> > > > >>>>>> > > > >>>>> > > > >>>> > > > >>> > > > >> -- > > > >> > > > >> Sergei > > > > > > > > > > -- > > > > Sergei > > >
