I agree we just need to take care of it as reviewers to inform the PR authors.
On Thu, Sep 10, 2020 at 4:30 PM Ash Berlin-Taylor <a...@apache.org> wrote: > Yeah, that's what I do (or try to. No one is perfect.) > > I'm sure it's written down _somewhere_ but I've long since forgotten > where I picked up that habit from. It's almost an extention to point 5 > in https://chris.beams.io/posts/git-commit/#imperative (which we already > point to). > > -a > > On Sep 10 2020, at 4:24 pm, Jarek Potiuk <jarek.pot...@polidea.com> wrote: > > > Also on that, As far as I remember the "unwritten" rule that Ash > mentioned > > some time ago as far as I remember the "subject" of the commit is best if > > it completes the sentence: > > > > "This commit when merged ...." ("fixes this and that" for example). > > > > And then the context on why we are doing it should follow. For some > > time I > > try to follow this quite rigorously with some of the heavier changes :) > > > > Do I remember it well Ash? > > > > J, > > > > > > On Thu, Sep 10, 2020 at 5:07 PM Tomasz Urbaszek < > tomasz.urbas...@polidea.com> > > wrote: > > > >> I agree with Jarek - as long as we cannot enforce it we as reviewers > >> should do our best to have the description in place. > >> > >> On a general note, we should always remember that even if we know each > >> other and communicate through different channels (slack, calls, etc) > >> other people won't know the context of the change if there's no > >> description. > >> > >> Tomek > >> > >> > >> On Thu, Sep 10, 2020 at 4:50 PM Jarek Potiuk <jarek.pot...@polidea.com> > >> wrote: > >> > > >> > I am afraid people won't read it anyway. the PR template should be as > >> > short as possible. I think this one is really something that should > >> > be on reviewers. It's one of those things that is hard to automate or > >> > delegate. > >> > > >> > If the reviewer does not understand what the PR from the description, > >> > it should be the first point to say "please provider context, I am not > >> > sure what the change does" even before looking at the code IMHO. If we > >> > all do that - this will be much more effective than writing it in the > >> > template. > >> > > >> > J > >> > > >> > On Thu, Sep 10, 2020 at 3:14 PM Ry Walker <r...@rywalker.com> wrote: > >> > > > >> > > Maybe modify the template to include some copy from your email in > >> it :) > >> > > > >> > > On Sun, Sep 6, 2020 at 6:56 AM Kaxil Naik <kaxiln...@gmail.com> > wrote: > >> > > > >> > > > Hi all, > >> > > > > >> > > > I have brought this topic on multiple occasions earlier too on the > >> mailing > >> > > > list. I sincerely request all the contributors and Committers > >> (including > >> > > > myself) that we add PR descriptions. > >> > > > > >> > > > This helps the community understand what the PRs do and abides by > >> the ASF > >> > > > motto about "Community above Code", many users lands to the PR > after > >> > > > checking change log and having to look at the code to > >> understand the > >> PR is > >> > > > not ideal. Adding descriptions of "most" of the PRs literally does > >> not take > >> > > > more than a minute. > >> > > > > >> > > > We previously had automation around it but we removed because of > >> small > >> > > > exceptions where the PR title can be self explanatory. > >> > > > > >> > > > We should not ignore rules (I am talking about PR template that > says > >> " ^ > >> > > > Add meaningful description above ". Again it is fine if the PR > title > >> is > >> > > > self explanatory but not otherwise. > >> > > > > >> > > > Regards, > >> > > > Kaxil > >> > > > > >> > > >> > > >> > > >> > -- > >> > > >> > Jarek Potiuk > >> > Polidea | Principal Software Engineer > >> > > >> > M: +48 660 796 129 > >> > >> > >> > >> -- > >> > >> Tomasz Urbaszek > >> Polidea | Software Engineer > >> > >> M: +48 505 628 493 > >> E: tomasz.urbas...@polidea.com > >> > >> Unique Tech > >> Check out our projects! > >> > > > > > > -- > > > > Jarek Potiuk > > Polidea <https://www.polidea.com/> | Principal Software Engineer > > > > M: +48 660 796 129 <+48660796129> > > [image: Polidea] <https://www.polidea.com/> > > >
