On 06/09/15 03:20, Yao, Jiewen wrote: > HI Laszlo > Thanks for your feedback. It reminds us to use GIT. > I am sorry that I missed your feedback before. I search it in my mailbox > again today, but I fail to find it. > Not sure why. :-( > > Currently we are still using SVN on some machines. Sorry to bring trouble for > GIT user. > We are moving towards GIT, and we are involving community for more discussion.
Works for me, thank you. I just hope that eventually everyone on the Intel firmware team will move to git. (I do realize there's the other discussion about submodules / subtrees, which could be a blocker for you guys.) Thanks Laszlo > > Thank you > Yao Jiewen > > -----Original Message----- > From: Justen, Jordan L > Sent: Tuesday, June 09, 2015 8:14 AM > To: edk2-devel@lists.sourceforge.net; Yao, Jiewen; Laszlo Ersek; Kinney, > Michael D > Subject: Re: [edk2] [patch] MdePkg/PropertiesTable support, > MdeModulePkg/DxeCore support UEFI2.5 properties table > > On 2015-06-04 10:47:37, Laszlo Ersek wrote: >> Jiewen, >> >> On 06/04/15 16:34, Yao, Jiewen wrote:> Hi >>> >>> Here is patch to support UEFI2.5, chapter 4, section 4.6 Properties >>> Table feature. >>> >>> MdePkg \u2013 add Properties table definition. >>> MdeModulePkg \u2013 add properties table support in DXE core. >>> MdeModulePkg \u2013 add PropertiesTableAttributesDxe driver to set >>> ACPINvs/Reserved memory type to be XP. >>> >>> Signed-off-by: Yao, Jiewen jiewen....@intel.com >>> <mailto:jiewen....@intel.com> >>> Reviewed-by: Zeng, Star star.z...@intel.com >>> <mailto:star.z...@intel.com> >>> Reviewed-by: Gao, Liming liming....@intel.com >>> <mailto:liming....@intel.com> >> >> Honest question: do you ever intend to abandon the following bad >> practices: >> - extremely long lines, >> - huge code bombs in a few patches, >> - posting patches as attachments? >> >> The diffstat for the second attachment is: >> >> /PropertiesTableAttributesDxe/PropertiesTableAttributesDxe.uni >> | 1 >> /PropertiesTableAttributesDxe/PropertiesTableAttributesDxeExtra.uni >> | 1 >> Core/Dxe/DxeMain.h >> | 29 >> Core/Dxe/DxeMain.inf >> | 4 >> Core/Dxe/DxeMain/DxeMain.c >> | 2 >> Core/Dxe/Image/Image.c >> | 2 >> Core/Dxe/Misc/PropertiesTable.c >> | 1418 ++++++++++ >> MdeModulePkg.dec >> | 22 >> MdeModulePkg.dsc >> | 2 >> Universal/PropertiesTableAttributesDxe/PropertiesTableAttributesDxe.c >> | 412 ++ >> Universal/PropertiesTableAttributesDxe/PropertiesTableAttributesDxe.inf >> | 114 >> Universal/PropertiesTableAttributesDxe/PropertiesTableAttributesDxe.uni >> | 1 >> >> Universal/PropertiesTableAttributesDxe/PropertiesTableAttributesDxeExtra.uni >> | 1 >> 13 files changed, 2009 insertions(+) >> >> Also, the longest line in the patch is 214 characters. >> >> Do you ever intend to adopt inline patches, to break up features into >> series of patches, and to follow the line length hints of the edk2 >> coding style? >> >> Honestly, I have the impression about these postings that they are an >> after-the-fact (non-)courtesy to the mailing list. "We wrote this at >> Intel, it has been reviewed, it's going in, we'll ignore your >> formatting requests that you've repeated many times; if you are unable >> to review it as-is, your loss". >> >> If you are committed to ignore these bugs in your patches in the long >> term, I'd like to know that. > > Jiewen, it seems like Laszlo expressed some (valid) concerns with your patch, > but you didn't respond, and checked them in anyway. > > Considering how much effort it is for people to code review patches, it is > not very friendly to ignore their feedback. > > Occasionally, in urgent cases, we can disagree and still move forward even > when there is disagreement, but I don't think it is good to ignore the > discussion entirely and move forward with no regard for the concerns. > > Did you intend this edk2-devel "code-review" to actually be "We wrote this at > Intel, it has been reviewed, it's going in" as was Laszlo's concern? > > I think the real start of the issue is that this should have been posted to > edk2-devel before there was any Reviewed-by. And, following the posting, all > feedback from the community should be considered. > > Mike, do you have any thoughts on how we might be able to address these > issues? Maybe we could consider removing commit privileges for developers > that don't follow the process? Is there someone else on the team besides you > that I should ask this question to? > > Thanks, > > -Jordan > ------------------------------------------------------------------------------ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
