> > While doing this I tried to come up with some labels that we can add to the > PRs (in-line with what we had on JIRA and our Changelog sections): > > > - type: bug-fix > - type: new-feature > - type: doc-change > - type: improvement > - type: internal [For all CI & tests related changes] >
I like this! Why don't we change our bot to make sure that one of those is applied before we merge? I think that fits very well the "review time" categorisation and we can indeed expect that committers can properly assign one of those. I agree that before 2.0 it's not a good idea to introduce semantic versioning and labels seem much nicer as they can be applied even retroactively. > After merging a PR if we (committers) can add one of this label and a > relevant *Milestone *(1.10.11 > <https://github.com/apache/airflow/milestone/4> for now - Don't need to > add > for 2.0 as it would be branched off the Master and generating Changelog for > 2.0 is a separate issue but let's deal with it at a later time). We should > only add a PR to this Milestone if we want a PR/commit to be cherry-picked > in Airflow 1.10.11 > See above ^^ - why not to check it BEFORE merge and make it prerequisite :). > > In the coming days: I will update our current release script to support > this and generate a Changelog from it we all agree to the labels I > suggested above. > I am good with the list. Consider it my +1. I am happy to also help with applying the labels to all 1.10.11 merged prs. re: *Commit Messages (and PR descriptions)* > > This is a tough one - I had changed the words when adding few commits to a > Changelog for 1.10.10 so that they are *more readable*. > > This would be tough to enforce and I feel changing the name before merging > would be our best bet. Most of the time the PR authors don't know (or > wouldn't know) what is the best way to describe the change. > I have re-written PR title on couple of occasions and I know many of other > committers have the done the same which I think is better than enforcing a > Language Parsing Bot" > Ok. Fine for me. I did few corrections myself, but it would be great to agree the rule for committers as well what the subject should read - the rule from Ash "When this commit is applied it will ...". Maybe we should have some kind of small checklist for committers to guide everyone (including future new committers?). I am happy to start it (but I wonder what would be the best place for it) > re: *Conventional Commits* > > > *tl;dr*: I am in favor of Conventional commits & Semantic commit messages > once we branch of the Master (for Airflow 2.0). > Yeah. That's good. Let's see if the label approach works - and maybe then we won't have to adopt it more. I think bot-gated label presence should do the trick even nicer - still distributing the work but among the committers rather than among contributors. > On Sun, Apr 26, 2020 at 6:49 PM Tomasz Urbaszek <turbas...@apache.org> > wrote: > > > We probably can try to add proper labels using a semantic prefix. As per > > irrelevant (from changelog point of view) we can use ci or dev prefixes. > > > > T. > > > > On Sun, Apr 26, 2020 at 6:48 PM Jarek Potiuk <jarek.pot...@polidea.com> > > wrote: > > > > > Reno sounds like it's there to handle the corporate environment with > > > multiple teams working on different features. A bit too much overhead > if > > > you ask me :). There is a reason why I am not working for a big > > corporation > > > :D > > > > > > I agree that Updating.md is fine as it is now. Also, I think often > > > Updating.md description is longer than one that will acceptably fit > into > > > commit message (and it might not have the same limitations for line > > > length). > > > > > > I think we should not necessarily publish what has been generated > > > automatically without modifications. I thought that we should generate > it > > > and review/manually update it afterwards. We even might want to remove > > some > > > of the stuff like dev/ci/chores etc. automatically when generating it. > I > > > think that's what your current script did so far? I believe (remember > > some > > > discussions with you or Kaxil) the cleanup/removal of CI/Chore stuff > was > > > quite some manual work. > > > > > > If we go semantic and make sure we review also the type, we can > > distribute > > > the "review" part among the committers and do it at the time of > review. I > > > believe it's much easier to verify type while you are reviewing the > > change > > > rather than when you prepare the release sometime few weeks or months > > later > > > (and you might have no idea what the change was about initially as it > > could > > > be approved by another committer). > > > > > > We could rely on labels of course, but those are a bit volatile and > they > > > leave no trace in git, Labels are github stuff and you need to query > for > > it > > > additionally with github API to get the label. Having a type of issue > in > > > the commit message I think simplifies the initial generation of such > > > changelog - you only rely on the .git content. > > > > > > J. > > > > > > > > > On Sun, Apr 26, 2020 at 6:04 PM Ash Berlin-Taylor <a...@apache.org> > > wrote: > > > > > > > Random other thoughts about changelog: > > > > > > > > Sometimes we get the type wrong (say it's a new feature when it's a > > fix) > > > > > > > > Some commits just don't need to be included in the changelog as they > > > > aren't relevant to users of Airflow. > > > > > > > > - Either it's a bug fix to a new feature that hasn't yet been > released > > > > - or its just internal and doesn't affect users when they install > > > Airflow. > > > > > > > > (To me the Changelog is for users seeing what has changed before > > > > upgrading. So things like CI improvements don't belong in there as > it's > > > not > > > > for them.) > > > > > > > > Not sure my point here, just thoughts of what I consider when > building > > a > > > > changelog for a release. > > > > > > > > -a > > > > > > > > On 26 April 2020 16:56:42 BST, Ash Berlin-Taylor <a...@apache.org> > > wrote: > > > > >Yes thank you, that's the one! > > > > > > > > > >On 26 April 2020 16:44:50 BST, "Kamil Breguła" > > > > ><kamil.breg...@polidea.com> wrote: > > > > >>Reno was developed by OpenStack: > > > > >>https://docs.openstack.org/reno/latest/ > > > > >> > > > > >>On Sun, Apr 26, 2020 at 5:39 PM Ash Berlin-Taylor <a...@apache.org> > > > > >>wrote: > > > > >>> > > > > >>> Yeah, probably is overly complex for general changelog. > > > > >>> > > > > >>> The main thing updating.md needs is some concept of knowing which > > > > >>release it is in when the commit appears in multiple branches (but > > > > >with > > > > >>different commit IDs because of cherry picking.) Sure we could > write > > > > >>the tooling for that, but someone already has. > > > > >>> > > > > >>> (I think it's for Openstack? I'll find the link on Monday) > > > > >>> > > > > >>> I'd say let's keep updating.md out of the discussion for now > > > > >>> > > > > >>> On 26 April 2020 16:15:18 BST, Jarek Potiuk > > > > >><jarek.pot...@polidea.com> wrote: > > > > >>> >> > > > > >>> >> > > > > >>> >> For changelog there is another option, which is a more > > > > >>formal/complex > > > > >>> >> system which works better with our backport/release branch > > > > >>process. > > > > >>> >(Can't > > > > >>> >> find the specific tool I'm thinking of from my phone. It > > involves > > > > >>> >each > > > > >>> >> change note in a separate file, and then a script to compile > it. > > > > >>> >Roughly) > > > > >>> >> That may be better suited to UPDATING.md though > > > > >>> >> > > > > >>> > > > > > >>> >I think overhead for that would be very, very painful for > regular > > > > >>> >changes. > > > > >>> >We already have the commit message - so why don't we simply use > it > > > > >>> >better. > > > > >>> > > > > > >>> >BTW. The commitizen + semantic commits already have a way to > > handle > > > > >>> >BREAKING CHANGES- they are put in the footer with "breaking > > > > >changes" > > > > >>> >prefix > > > > >>> >and you can put the right description there (and it can be used > to > > > > >>add > > > > >>> >them > > > > >>> >to changelog/updating.md updates). Here I am on the fence if we > > > > >>should > > > > >>> >use > > > > >>> >it or not - because `git annotate` on UPDATING.md has already > all > > > > >>info > > > > >>> >that > > > > >>> >is needed, but maybe we can discuss the approach here as well. > > > > >>> > > > > > >>> >J. > > > > >>> > > > > > >>> > > > > > >>> > > > > > >>> >> > > > > >>> >> -a > > > > >>> >> > > > > >>> >> On 26 April 2020 14:38:17 BST, Tomasz Urbaszek > > > > >><turbas...@apache.org> > > > > >>> >> wrote: > > > > >>> >> >I agree with Ash that it's a committer's task to check the > > > > >commit > > > > >>> >name. > > > > >>> >> >But > > > > >>> >> >I personally was deceived by a discrepancy between PR name > and > > > > >>> >commit > > > > >>> >> >title > > > > >>> >> >and merged the PR with "wrong" commit message. > > > > >>> >> > > > > > >>> >> >That's where I agree with Jarek: we should help ourselves. If > > > > >>"red > > > > >>> >> >check" > > > > >>> >> >will make people correct the commit / PR title then there > will > > > > >be > > > > >>> >less > > > > >>> >> >work > > > > >>> >> >for us and fewer mistakes made by us. Also, semantic commits > > > > >have > > > > >>a > > > > >>> >> >nice > > > > >>> >> >side effect: they teach about commit messages. I think > having a > > > > >>tool > > > > >>> >to > > > > >>> >> >check and teach is better having a "you should follow this" > > > > >link, > > > > >>> >which > > > > >>> >> >in > > > > >>> >> >most cases is ticked without clicking the link (btw. should > we > > > > >>> >measure > > > > >>> >> >it?). > > > > >>> >> > > > > > >>> >> >One of my reason to suggest it was a conventional changelog > > that > > > > >>> >could > > > > >>> >> >be > > > > >>> >> >auto-generated. As Jarek mentioned currently it's mostly done > > by > > > > >>> >@Kaxil > > > > >>> >> >and > > > > >>> >> >it would be interesting to hear what he thinks about it. > > > > >>> >> > > > > > >>> >> >T. > > > > >>> >> > > > > > >>> >> >On Sun, Apr 26, 2020 at 2:44 PM Jarek Potiuk > > > > >>> ><jarek.pot...@polidea.com> > > > > >>> >> >wrote: > > > > >>> >> > > > > > >>> >> >> I think you pointed out the exact things I thought are > > > > >>important > > > > >>> >and > > > > >>> >> >could > > > > >>> >> >> be automated. I think those are the very things committing > > > > >>checks > > > > >>> >> >for. > > > > >>> >> >> > > > > >>> >> >> I think we could benefit from 1, 2, 3, 4, 6 (6. with > > > > >exceptions > > > > >>> >> >indeed but > > > > >>> >> >> a warning or a way to mark an exception would be nice - > > > > >>similarly > > > > >>> >as > > > > >>> >> >we do > > > > >>> >> >> with pylint). > > > > >>> >> >> I certainly did not want to improve automatically on the 5 > > > > >>(yet) > > > > >>> >and > > > > >>> >> >7 > > > > >>> >> >> (here it's much more of a convention we agree between the > > > > >>> >committers > > > > >>> >> >- > > > > >>> >> >> whether the body should be optional - I think it should and > > > > >>> >whether > > > > >>> >> >it > > > > >>> >> >> should be opt-out rather than opt-in - I thin it should be > > > > >>> >opt-out). > > > > >>> >> >> > > > > >>> >> >> There are quite a few commits currently with ... when you > > look > > > > >>at > > > > >>> >> >> the commit log in Github for one (because they do not obey > > the > > > > >>> >> >subject > > > > >>> >> >> length) - I picked the ones without JIRAs - still even > > without > > > > >>> >JIRAs > > > > >>> >> >> sometimes the subject is too long: > > > > >>> >> >> > > > > >>> >> >> - > > > > >>> >> >> > > > > >>> >> >> > > > > >>> >> > > > > > >>> >> > > > > >>> > > > > >>> > > > > > > > > > > https://github.com/apache/airflow/commit/d883ff49ca2841f91ab7e0ab98204d5ad271473b > > > > >>> >> >> - > > > > >>> >> >> > > > > >>> >> >> > > > > >>> >> > > > > > >>> >> > > > > >>> > > > > >>> > > > > > > > > > > https://github.com/apache/airflow/commit/bc230a9711fec2004e20f46aee22fb44c7461b6c > > > > >>> >> >> - > > > > >>> >> >> > > > > >>> >> >> > > > > >>> >> > > > > > >>> >> > > > > >>> > > > > >>> > > > > > > > > > > https://github.com/apache/airflow/commit/fa262c12f87102a7ae1abb11ea7f0d5e8be0de47 > > > > >>> >> >> > > > > >>> >> >> However - this is secondary. It was merely a comment on the > > > > >>> >possible > > > > >>> >> >> completion of the "semantic convention" approach. This is > the > > > > >>main > > > > >>> >> >subject. > > > > >>> >> >> > > > > >>> >> >> I think the main idea behind the semantic commit/PR is the > > > > >>prefix > > > > >>> >is > > > > >>> >> >that > > > > >>> >> >> it allows for much easier and consistent ChangeLog > > generation. > > > > >>For > > > > >>> >> >example > > > > >>> >> >> in Angular you have > > > > >>> >> >> > https://github.com/angular/angular/blob/master/CHANGELOG.md > > > > >>which > > > > >>> >is > > > > >>> >> >> generated automatically including breaking changes etc. I > > > > >>think > > > > >>> >it's > > > > >>> >> >> mainly Kaxil's work now to prepare the changelog and group > > the > > > > >>> >> >changes into > > > > >>> >> >> separate buckets, so Kaxil - maybe your opinion is > important > > > > >>here. > > > > >>> >If > > > > >>> >> >there > > > > >>> >> >> is a way everyone as committers and contributors we can do > to > > > > >>make > > > > >>> >> >release > > > > >>> >> >> manager's job easier - I think we should do it. > > > > >>> >> >> > > > > >>> >> >> BTW. The convention is easy to follow without any tools. > > > > >>However > > > > >>> >> >commitizen > > > > >>> >> >> has the nice feature of also guiding new users - it > provides > > a > > > > >>> >nice > > > > >>> >> >> explanation of the types you have defined in the project > and > > > > >>guide > > > > >>> >> >the new > > > > >>> >> >> users how to write a good commit. I think it might be > really > > > > >>nice > > > > >>> >> >touch for > > > > >>> >> >> our "welcoming community" approach. See the 5 minutes video > > > > >>about > > > > >>> >it: > > > > >>> >> >> > > > > >>> >> >> > > > > >>> >> > > > > > >>> >> > > > > >>> > > > > >>> > > > > > > > > > > https://egghead.io/lessons/javascript-writing-conventional-commits-with-commitizen > > > > >>> >> >> > > > > >>> >> >> J. > > > > >>> >> >> > > > > >>> >> >> J. > > > > >>> >> >> > > > > >>> >> >> > > > > >>> >> >> > > > > >>> >> >> On Sun, Apr 26, 2020 at 2:13 PM Ash Berlin-Taylor > > > > >><a...@apache.org> > > > > >>> >> >wrote: > > > > >>> >> >> > > > > >>> >> >> > My main objection is this is trying to apply a technical > > > > >>> >solution > > > > >>> >> >to a > > > > >>> >> >> > people+English problem. This feels like just one extra > step > > > > >>to > > > > >>> >have > > > > >>> >> >> > commiters to do, when we as committers can very easily > > > > >>correct > > > > >>> >this > > > > >>> >> >in > > > > >>> >> >> > Github whilst reviewing/before merging. > > > > >>> >> >> > > > > > >>> >> >> > That said, can you point at any examples of recent > commits > > > > >>that > > > > >>> >you > > > > >>> >> >> > think would have been clearer as a result of using? > > > > >>> >> >> > > > > > >>> >> >> > (Also a significant proportion of commits from form a git > > > > >gui > > > > >>or > > > > >>> >an > > > > >>> >> >ide, > > > > >>> >> >> > so cz-cli won't help those users.) > > > > >>> >> >> > > > > > >>> >> >> > The "good commit messages" we already link to > > > > >>> >> >> > https://chris.beams.io/posts/git-commit/ has these > points > > > > >>> >> >> > > > > > >>> >> >> > 1. Separate subject from body with a blank line > > > > >>> >> >> > 2. Limit the subject line to 50 characters > > > > >>> >> >> > 3. Capitalize the subject line > > > > >>> >> >> > 4. Do not end the subject line with a period > > > > >>> >> >> > 5. Use the imperative mood in the subject line > > > > >>> >> >> > 6. Wrap the body at 72 characters > > > > >>> >> >> > 7. Use the body to explain what and why vs. how > > > > >>> >> >> > > > > > >>> >> >> > 2 we _could_ enforce, but it is not a hard-and-fast > rule. 5 > > > > >>and > > > > >>> >7 > > > > >>> >> >is > > > > >>> >> >> > almost impossible for a computer to enforce. 6 always has > > > > >>> >> >exceptions. > > > > >>> >> >> > The most important ones is 7, and that is the hardest to > > > > >>> >> >programitcally > > > > >>> >> >> > enforce. > > > > >>> >> >> > > > > > >>> >> >> > -a > > > > >>> >> >> > > > > > >>> >> >> > On Apr 26 2020, at 11:30 am, Jarek Potiuk > > > > >>> >> ><jarek.pot...@polidea.com> > > > > >>> >> >> > wrote: > > > > >>> >> >> > > > > > >>> >> >> > > I think it's a very good idea to use it. We already > > > > >>discussed > > > > >>> >> >that we > > > > >>> >> >> > > should have some improvements in the way we write > commits > > > > >- > > > > >>> >and > > > > >>> >> >why to > > > > >>> >> >> > > come up with our own conventions if we can adopt one > that > > > > >>> >already > > > > >>> >> >> > > exists and has set of nice tools available. > > > > >>> >> >> > > > > > > >>> >> >> > > As usual, I think automation is a key - as it might > make > > > > >>lives > > > > >>> >of > > > > >>> >> >> > > committers a bit easier. There are already a number of > > > > >>tools > > > > >>> >that > > > > >>> >> >we > > > > >>> >> >> > > could use together with such convention, as both > > > > >>pre-commits > > > > >>> >and > > > > >>> >> >a bot > > > > >>> >> >> > > in Github. > > > > >>> >> >> > > There are quite a few tools that embraced the concept > of > > > > >>> >semantic > > > > >>> >> >> > > pr/semantic commits and I have heard good words about > > them > > > > >>> >from > > > > >>> >> >other > > > > >>> >> >> > > open-source projects. I've heard especially good words > > > > >>about > > > > >>> >> >> > > commitizen CLI, that could work hand-in-hand with > > semantic > > > > >>> >> >> > > commits/PRs: > > > > >>> >> >> > > > > > > >>> >> >> > > https://github.com/commitizen/cz-cli > > > > >>> >> >> > > > > > > >>> >> >> > > One of the things it has it also integrates with commit > > > > >>lint > > > > >>> >> >where we > > > > >>> >> >> > > could write our own rules and make them more meaningful > > > > >>> >> >> > > https://commitlint.js.org/#/ > > > > >>> >> >> > > > > > > >>> >> >> > > Also, there are ready-to-use changelog generators that > we > > > > >>can > > > > >>> >use > > > > >>> >> >(for > > > > >>> >> >> > > example > > > > >>> >https://github.com/commitizen/cz-conventional-changelog ) > > > > >>> >> >> > > > > > > >>> >> >> > > Those are tools coming from the nodejs world, but I do > > not > > > > >>see > > > > >>> >a > > > > >>> >> >big > > > > >>> >> >> > > problem with using them (of course trying them out > first) > > > > >- > > > > >>> >since > > > > >>> >> >we > > > > >>> >> >> > > can now connect it via pre-commit, it should be easy to > > > > >add > > > > >>> >all > > > > >>> >> >that > > > > >>> >> >> > > to our toolbox. > > > > >>> >> >> > > > > > > >>> >> >> > > J. > > > > >>> >> >> > > > > > > >>> >> >> > > > > > > >>> >> >> > > On Sun, Apr 26, 2020 at 10:44 AM Ash Berlin-Taylor > > > > >>> >> ><a...@apache.org> > > > > >>> >> >> > wrote: > > > > >>> >> >> > >> > > > > >>> >> >> > >> I agree that many commit messages are often lacking > but > > > > >>I'm > > > > >>> >not > > > > >>> >> >a fan > > > > >>> >> >> > >> of that the prefix style that app requires, - plus I > > > > >think > > > > >>it > > > > >>> >> >would > > > > >>> >> >> > >> still be possible to have unhelpful PR titles just > with > > > > >>> >'fix:' > > > > >>> >> >> prefixed. > > > > >>> >> >> > >> > > > > >>> >> >> > >> Is rather we as commiters updated the pr subjects when > > > > >>> >> >reviewing. The > > > > >>> >> >> > >> rule I try to follow is to (mentally) prefix the > message > > > > >>with > > > > >>> >> >"When > > > > >>> >> >> > >> this commit is applied it will ..." > > > > >>> >> >> > >> > > > > >>> >> >> > >> -a > > > > >>> >> >> > >> > > > > >>> >> >> > >> On 26 April 2020 09:34:56 BST, Tomasz Urbaszek > > > > >>> >> ><turbas...@apache.org> > > > > >>> >> >> > wrote: > > > > >>> >> >> > >> >Hi all! > > > > >>> >> >> > >> > > > > > >>> >> >> > >> >Sometimes it happens that pull requests or commits > have > > > > >>not > > > > >>> >so > > > > >>> >> >> > >> >meaningful messages and it's hard to say what's > exactly > > > > >>> >going > > > > >>> >> >on. > > > > >>> >> >> > >> >So I am wondering if we would like to consider using > > > > >>> >semantic > > > > >>> >> >pull > > > > >>> >> >> > >> >request: > > https://github.com/zeke/semantic-pull-requests > > > > >>> >> >> > >> > > > > > >>> >> >> > >> >Since we are using Github it should be pretty easy to > > > > >>add: > > > > >>> >> >> > >> >https://github.com/apps/semantic-pull-requests > > > > >>> >> >> > >> > > > > > >>> >> >> > >> >Of course, it does not solve the problem of "pr > > message" > > > > >>but > > > > >>> >> >> > >> >definitely it raises attention about it. On the other > > > > >>hand, > > > > >>> >it > > > > >>> >> >should > > > > >>> >> >> > >> >also help with publishing changelogs. Personally I > like > > > > >>this > > > > >>> >> >approach > > > > >>> >> >> > >> >and I used to use it before joining Airflow. > > > > >>> >> >> > >> > > > > > >>> >> >> > >> >Happy to see what you think about it. And sorry if it > > > > >was > > > > >>> >> >decided > > > > >>> >> >> some > > > > >>> >> >> > >> >ago that Airflow won't follow it. > > > > >>> >> >> > >> > > > > > >>> >> >> > >> >Cheers, > > > > >>> >> >> > >> >Tomek > > > > >>> >> >> > > > > > > >>> >> >> > > > > > > >>> >> >> > > > > > > >>> >> >> > > -- > > > > >>> >> >> > > > > > > >>> >> >> > > Jarek Potiuk > > > > >>> >> >> > > Polidea | Principal Software Engineer > > > > >>> >> >> > > > > > > >>> >> >> > > M: +48 660 796 129 > > > > >>> >> >> > > > > > > >>> >> >> > > > > > >>> >> >> > > > > >>> >> >> > > > > >>> >> >> -- > > > > >>> >> >> > > > > >>> >> >> Jarek Potiuk > > > > >>> >> >> Polidea <https://www.polidea.com/> | Principal Software > > > > >>Engineer > > > > >>> >> >> > > > > >>> >> >> M: +48 660 796 129 <+48660796129> > > > > >>> >> >> [image: Polidea] <https://www.polidea.com/> > > > > >>> >> >> > > > > >>> >> > > > > >>> > > > > > >>> > > > > > >>> >-- > > > > >>> > > > > > >>> >Jarek Potiuk > > > > >>> >Polidea <https://www.polidea.com/> | Principal Software > Engineer > > > > >>> > > > > > >>> >M: +48 660 796 129 <+48660796129> > > > > >>> >[image: Polidea] <https://www.polidea.com/> > > > > > > > > > > > > > -- > > > > > > Jarek Potiuk > > > Polidea <https://www.polidea.com/> | Principal Software Engineer > > > > > > M: +48 660 796 129 <+48660796129> > > > [image: Polidea] <https://www.polidea.com/> > > > > > > -- Jarek Potiuk Polidea <https://www.polidea.com/> | Principal Software Engineer M: +48 660 796 129 <+48660796129> [image: Polidea] <https://www.polidea.com/>
