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/>
