public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Gao, Liming" <liming.gao@intel.com>
To: Evan Lloyd <Evan.Lloyd@arm.com>, Laszlo Ersek <lersek@redhat.com>,
	"Ni, Ruiyu" <ruiyu.ni@intel.com>
Cc: "edk2-devel (edk2-devel@lists.01.org)" <edk2-devel@lists.01.org>
Subject: Re: Incorrect Author on patch
Date: Wed, 2 May 2018 15:20:03 +0000	[thread overview]
Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14E22344D@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <DB6PR08MB2806CEA148FC101FC1A1C1818B800@DB6PR08MB2806.eurprd08.prod.outlook.com>

Evan:
  I agree this is a mistake in edk2 project. We mix the operation in edk2 project and our internal project. We will double check the patch and avoid such case happen again.

Thanks
Liming
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Evan Lloyd
> Sent: Wednesday, May 2, 2018 8:46 PM
> To: Laszlo Ersek <lersek@redhat.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> Cc: edk2-devel (edk2-devel@lists.01.org) <edk2-devel@lists.01.org>
> Subject: Re: [edk2] Incorrect Author on patch
> 
> One obvious way of precluding this sort of problem would be to move to using the pull request mechanism on GitHub, rather than
> requiring maintainers to play with upstreaming local patches.  I can well understand why it would be useful to use Gerrit as a means of
> reviewing a patch - actually a brilliant idea, but it does introduce problems. If, post review, the merge were  to be from a pull request
> then there would be no risk of "meta-data corruption".
> 
> Regards
> Evan
> 
> > -----Original Message-----
> > From: Laszlo Ersek <lersek@redhat.com>
> > Sent: 25 April 2018 22:15
> > To: Evan Lloyd <Evan.Lloyd@arm.com>; ruiyu.ni@intel.com
> > Cc: edk2-devel (edk2-devel@lists.01.org) <edk2-devel@lists.01.org>
> > Subject: Re: [edk2] Incorrect Author on patch
> >
> > On 04/25/18 17:02, Evan Lloyd wrote:
> > > Hi Ruiyu.
> > > When we look at the edk2 git log, or GitHub
> > >
> > https://github.com/tianocore/edk2/commit/ee4dc24f57c32a445e7c747396c
> > 9b
> > > fbd8b221568 we see that Sami's AcpiView patch shows you as the Author.
> > > I'm not sure what might have caused that (and it is obviously accidental), but
> > it is a little unfortunate in that the commit doesn't show up on Sami's GitHub
> > stats.
> > > Fortunately, this is not currently significant for our organisation, although I'm
> > sure Sami would prefer to have the credit.
> > > This is a trivial matter, but you may wish to understand what caused it so that
> > you don't accidentally upset someone for whom the stats are significant.
> >
> > Right, this occurred at least one other time as well: see commit
> > 8b5c80e0296c ("MdeModulePkg/UefiBootManagerLib: fix
> > AddLoadOptionVariable docs/prototype", 2018-04-23). Ray pushed the patch
> > (so the Committer field is certainly right), but Ray's name+email replaced mine
> > in the Author field. I had noticed that earlier, but now I'm seeing it as a pattern.
> >
> > I believe this is a tooling issue. I notice the following bit on the commit
> > message:
> >
> >     Change-Id: I8a609d6502b6f8929b2f568acaa147065003b6f4
> >
> > I certainly didn't post the patch with that, and I doubt Ray added it manually.
> > So, whatever tool Ray used to handle the patch lost the authorship
> > information.
> >
> > And, I see the exact same kind of tag, namely
> >
> >     Change-Id: Ifa23dc80ab8ab042c56e88424847e796a8122a7c
> >
> > on commit ee4dc24f57c3 ("ShellPkg: Add acpiview tool to dump ACPI tables",
> > 2018-04-23), which you mention.
> >
> > ... In fact, my patch happens to be the direct ancestor of Sami's, in the git
> > history, and their commit times are just ~3 minutes apart. I'm quite certain Ray
> > has started using a new tool.
> >
> > For example, commit bc2288f59ba2 ("UefiCpuPkg/MpInitLib: put
> > mReservedApLoopFunc in executable memory", 2018-03-08) was also
> > committed by Ray, *but* Jian's authorship was preserved fine. (No "Change-
> > Id" either.)
> >
> > ... Oh... it looks like those Change-Id's were added by Gerrit:
> >
> > https://git.eclipse.org/r/Documentation/user-changeid.html
> >
> > And then, please look at this:
> >
> > https://gerrit-review.googlesource.com/Documentation/error-invalid-
> > author.html
> >
> >     For every pushed commit Gerrit verifies that the e-mail address of
> >     the author matches one of the registered e-mail addresses of the
> >     pushing user. If this is not the case pushing the commit fails with
> >     the error message "invalid author". This policy can be bypassed by
> >     having the access right 'Forge Author'.
> >
> > Uh, what?... Gerrit says "invalid author" if Ray pushes a patch that wasn't
> > authored by himself? And the option to override that is called "forge" author?
> > O_o
> >
> > Anyway, the page continues,
> >
> >     If pushing to Gerrit fails with the error message "invalid author"
> >     and somebody else is author of the commit for which the push fails,
> >     then you have no permissions to forge the author identity. In this
> >     case you may contact the project owner to request the access right
> >     '+1 Forge Author Identity' in the 'Forge Identity' category or ask
> >     the maintainer to commit this change on the author’s behalf.
> >
> > Ray, if you absolutely must use Gerrit, please make sure that you have the '+1
> > Forge Author Identity' access right. IMO, we maintainers are responsible for
> > preserving git metadata the best we can.
> >
> > (For example, if a patch is applied from the mailing list with "git am", it
> > preserves the authorship information -- the documentation says, 'The commit
> > author name is taken from the "From: " line of the message, and commit
> > author date is taken from the "Date: " line of the message.')
> >
> > Thank you!
> > Laszlo
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the
> intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose,
> or store or copy the information in any medium. Thank you.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

      parent reply	other threads:[~2018-05-02 15:20 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-25 15:02 Incorrect Author on patch Evan Lloyd
2018-04-25 21:14 ` Laszlo Ersek
2018-05-02 12:46   ` Evan Lloyd
2018-05-02 13:40     ` Laszlo Ersek
2018-05-02 18:31       ` Rebecca Cran
2018-05-02 15:20     ` Gao, Liming [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4A89E2EF3DFEDB4C8BFDE51014F606A14E22344D@SHSMSX104.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox