Re: reagarding backport information while porting patches

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On Fri, Jun 23, 2017 at 3:01 PM, Niels de Vos <ndevos@xxxxxxxxxx> wrote:
On Fri, Jun 23, 2017 at 01:47:57PM +0530, Pranith Kumar Karampuri wrote:
> On Fri, Jun 23, 2017 at 1:30 PM, Niels de Vos <ndevos@xxxxxxxxxx> wrote:
>
> > On Fri, Jun 23, 2017 at 09:15:21AM +0530, Pranith Kumar Karampuri wrote:
> > > hi,
> > >      Now that we are doing backports with same Change-Id, we can find the
> > > patches and their backports both online and in the tree without any extra
> > > information in the commit message. So shall we stop adding text similar
> > to:
> > >
> > >     > Reviewed-on: https://review.gluster.org/17414
> > >     > Smoke: Gluster Build System <jenkins@xxxxxxxxxxxxxxxxx>
> > >     > Reviewed-by: Pranith Kumar Karampuri <pkarampu@xxxxxxxxxx>
> > >     > Tested-by: Pranith Kumar Karampuri <pkarampu@xxxxxxxxxx>
> > >     > NetBSD-regression: NetBSD Build System <jenkins@xxxxxxxxxxxxxxxxx>
> > >     > Reviewed-by: Amar Tumballi <amarts@xxxxxxxxxx>
> > >     > CentOS-regression: Gluster Build System <jenkins@xxxxxxxxxxxxxxxxx
> > >
> > >     (cherry picked from commit de92c363c95d16966dbcc9d8763fd4448dd84d13)
> > >
> > > in the patches?
> > >
> > > Do you see any other value from this information that I might be missing?
> >
> > I think it is good practise to mention where the backport comes from,
> > who developed and reviewed the original. At least the commit-id is
> > important, that way the backport can easily be compared to the original.
> > git does not know about Change-Ids, but does know commmit-ids :)
> >
>
> Change-ID captures all this information. You can know the patch in all
> branches with Change-ID, now that we are following same Change-ID across
> branches.

However a Change-Id is not standard git, it is purely a Gerrit thing. I
can't cherry-pick or 'git show' a Change-Id, but that works fine with a
git-commit.

Where do we mention git commit-id now? If we do the backport using gerrit, it adds it as "(cherry picked from commit de92c363c95d16966dbcc9d8763fd4448dd84d13)", otherwise I don't think it gets mentioned, right?
 

I also like to give credits to the people that originally wrote and
reviewed the change. It is not required, but it is nice to have.

If you do the backport using standard commands Author and committer are correctly shown in the patch, I think the tool already handles it. As for the reviewer etc. as this information is available on the actual patches, if the person who is checking for it really wants it, can find out.
 

> > We should try to have all the needed details in the git repository, and
> > not rely on Gerrit for patch verification/checking. When I'm working on
> > a patch and wonder why/when something related was changed, I'll use the
> > local history, and do not want to depend on Gerrit.
> >
>
> Change-ID is mentioned in the commit which is there in the git log, so we
> can figure out the information without needing internet/gerrit. So that
> part is not a problem.

Yes, of course I can figure it out, but it is additional work. Most
tools do not know about Change-Ids, like browsing through the code on
GitHub; a commit-id will be linked, a Change-Id not.


We will discuss this further after my question above about commit-id is answered.
 
> All of this information was important to mention earlier because there was
> no common thing binding all together. With same Change-ID across branches
> for a patch, it seems unnecessary to mention this information in the commit
> message.

Not everyone is familiar with how Gerrit handles Change-Ids. A
git-commit is something that other (new) contributors understand better.
I prefer to make it as easy as possible for people to go through the git
log and compare/check/verify whatever they are looking for. Limiting
references to Change-Ids makes their task a little more difficult.

Same here: We will discuss this further after my question above about commit-id is answered.
 

Niels



--
Pranith
_______________________________________________
Gluster-devel mailing list
Gluster-devel@xxxxxxxxxxx
http://lists.gluster.org/mailman/listinfo/gluster-devel

[Index of Archives]     [Gluster Users]     [Ceph Users]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux