From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) (using TLSv1 with cipher CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 5B6D11A1DF6 for ; Wed, 7 Sep 2016 10:39:24 -0700 (PDT) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga104.fm.intel.com with ESMTP; 07 Sep 2016 10:39:23 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,296,1470726000"; d="scan'208";a="5568270" Received: from jjloucai-mobl1.amr.corp.intel.com (HELO localhost) ([10.252.131.171]) by fmsmga005.fm.intel.com with ESMTP; 07 Sep 2016 10:39:23 -0700 MIME-Version: 1.0 To: "Yao, Jiewen" , "Yao, Jiewen" , "edk2-devel@lists.01.org" Message-ID: <147326996342.12894.6103663724254201876@jljusten-ivb> From: Jordan Justen In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C50385FAA16@shsmsx102.ccr.corp.intel.com> Cc: "Chan, Amy" , "Yao, Jiewen" References: <1473211270-12320-1-git-send-email-jiewen.yao@intel.com> <147323211068.9581.7670554499055950051@jljusten-ivb> <74D8A39837DF1E4DA445A8C0B3885C50385FA9CA@shsmsx102.ccr.corp.intel.com> <74D8A39837DF1E4DA445A8C0B3885C50385FAA16@shsmsx102.ccr.corp.intel.com> User-Agent: alot/0.3.7 Date: Wed, 07 Sep 2016 10:39:23 -0700 Subject: Re: [PATCH] Maintainers.txt: Add Giri as 2nd maintainer X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 07 Sep 2016 17:39:24 -0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On 2016-09-07 01:00:46, Yao, Jiewen wrote: > Jordan > = > I have a quick check: > = > Here is my observation: > = > 1) From > https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-F= ormat, > it mentions: > = > o The length of 'Pkg-Module: Brief-single-line-summary' should not > exceed 70 characters > = > 2) In the PatchCheck.py, we have below code: > = > if count >=3D 1 and len(lines[0]) > 76: > = > self.error('First line of commit message (subject line) ' + > = > 'is too long.') > = I think the general idea of the subject line is described well here as the "summary phrase": http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documen= tation/SubmittingPatches?id=3Dbca47613#n493 Regarding subject line length, I think that it should work well with git log --oneline to produce output less than 80 characters. Based on the standard prefix in git log --oneline, this would indicate that we should try to use less than 72 characters. I guess the kernel uses 70-75 characters for guidance. > = > 3) You recommendation > =E2=80=9CMaintainers.txt: Add Giri as IntelFsp2*Pkg, IntelSiliconPkg m= aintainer=E2=80=9D, > it is 71 char. > = > However, if we add =E2=80=9C[edk2] [PATCH]=E2=80=9D, it becomes 85 cha= r. > = Hmm, I don't think "[edk2] [PATCH]" should be factored into the length. The length after applying in git is what should matter. Is this is a bug in PatchCheck.py? I think maybe the subject line length could be a warning from PatchCheck.py, but it is almost always possible to reword the subject line fairly easily. If it is difficult to make a good short subject line, then it might be a sign that the patch should be split. For example, you could split this patch in 2. One for IntelFsp2*Pkg, and another for IntelSiliconPkg. (After all, they are 2 separate package types, so it is reasonable to change them separately.) -Jordan > = > I am confused on the rule. So I have 2 suggestion: > = > 1) wiki mentions *70* char, but tool checks *76* char. It seems > mismatch. We had better fix that. Can we make them consistent? > = > 2) In most cases, the patch subject is start with *[edk2] [PATCH]= *. > These 14 additional char had better be excluded in the total 70 or 76 = char > (I am not sure what is right number), when the subject is calculated. > = > I have no problem to fix this patch manually. > = > I do hope we can have consistent rule and tool to help us do the check. > = > Thank you > = > Yao Jiewen > = > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Yao, Jiewen > Sent: Wednesday, September 7, 2016 3:21 PM > To: Justen, Jordan L ; edk2-devel@lists.01.= org > Cc: Chan, Amy > Subject: Re: [edk2] [PATCH] Maintainers.txt: Add Giri as 2nd maintainer > = > = > = > Jordan > I got error from BaseTools\Scripts\PatchCheck.py, if I add it. > = > Can you fix the tool to remove such limitation? > = > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > Checking patch file: C:\home\EdkIITransition\AllComboGit\edk2\0001-Mai= ntainers.t > xt-Add-Giri-as-2nd-maintainer-to-IntelF.patch > The commit message format is not valid: > * First line of commit message (subject line) is too long. > https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-F= ormat > The code passed all checks. > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > = > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of= Jordan Justen > Sent: Wednesday, September 7, 2016 3:09 PM > To: Yao, Jiewen ; edk2-devel@lists.01.org > Cc: Chan, Amy > Subject: Re: [edk2] [PATCH] Maintainers.txt: Add Giri as 2nd maintainer > = > What about this patch subject instead? > = > Maintainers.txt: Add Giri as IntelFsp2*Pkg, IntelSiliconPkg maintainer > = > This way we can see the affected packages from the subject line. > = > -Jordan > = > On 2016-09-06 18:21:10, Jiewen Yao wrote: > > Add Giri as 2nd maintainer to IntelFsp2*Pkg and IntelSiliconPkg. > > > > Cc: Giri P Mudusuru > > > Cc: Amy Chan > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Jiewen Yao > > > --- > > Maintainers.txt | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/Maintainers.txt b/Maintainers.txt > > index b2e679d..6ac7085 100644 > > --- a/Maintainers.txt > > +++ b/Maintainers.txt > > @@ -130,10 +130,12 @@ M: Jeff Fan > > > IntelFsp2Pkg > > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2P= kg > > M: Jiewen Yao > > > +M: Giri P Mudusuru > > > > > IntelFsp2WrapperPkg > > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2W= rapperPkg > > M: Jiewen Yao > > > +M: Giri P Mudusuru > > > > > IntelFspPkg > > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFspPkg > > @@ -146,6 +148,7 @@ M: Jiewen Yao > > > IntelSiliconPkg > > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelSilic= onPkg > > M: Jiewen Yao > > > +M: Giri P Mudusuru > > > > > MdeModulePkg > > W: https://github.com/tianocore/tianocore.github.io/wiki/MdeModuleP= kg > > -- > > 2.7.4.windows.1 > > > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel