public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Yao, Jiewen" <jiewen.yao@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
	"lersek@redhat.com" <lersek@redhat.com>,
	"gaoliming@byosoft.com.cn" <gaoliming@byosoft.com.cn>,
	"Dong, Guo" <guo.dong@intel.com>
Cc: "marcello.bauer@9elements.com" <marcello.bauer@9elements.com>,
	"Kinney, Michael D" <michael.d.kinney@intel.com>,
	"'Leif Lindholm (Nuvia address)'" <leif@nuviainc.com>,
	"Doran, Mark" <mark.doran@intel.com>,
	'Andrew Fish' <afish@apple.com>,
	"Guptha, Soumya K" <soumya.k.guptha@intel.com>
Subject: Re: [edk2-devel] more development process failure [was: UefiPayloadPkg: Runtime MMCONF]
Date: Thu, 17 Sep 2020 07:31:23 +0000	[thread overview]
Message-ID: <CY4PR11MB1288B90FC46FB96DFA5642B28C3E0@CY4PR11MB1288.namprd11.prod.outlook.com> (raw)
In-Reply-To: <31e807dc-6217-f3b6-995b-ab10f4ce789e@redhat.com>

Laszlo
I like your description to compare the process with the programming language and software design. We have to clean up the resource.

Please allow me to point out, this is the exactly the issue we are having today.

1) Take buffer overflow as an example. It has been there for 30 years. We have enough books and papers talking about it. Today, people are still making mistakes. Why? Because C language is not type safe, there is NO enforcement.
That is why many people like other type safe language. If you are trying to make mistake, the compiler will tell you that you are making mistake.

2) Take resource leak as an example. The programming language invented garbage collection. The operating system auto cleaned up application resource after execution.

3) People has wrong check in which may break the system. What is why the world invented smoke test and continuous integration.

*Those are where the inventions come*, to treat human as human being, to prevent people making mistake or teach them automatically. :-)

I agree with the "mythical man-month" that there is no silver bulletin.
I tend to agree with you on the attitude.
I am trying to figure if we can do better to help other people (maintainer or contributor). If we really really can do nothing, that is OK.
I am not sure if is a best way to resolve the problem to just complain in the email.

I think you can understand my point. I will stop here.

Thank you
Yao Jiewen


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
> Sent: Thursday, September 17, 2020 2:32 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io;
> gaoliming@byosoft.com.cn; Dong, Guo <guo.dong@intel.com>
> Cc: marcello.bauer@9elements.com; Kinney, Michael D
> <michael.d.kinney@intel.com>; 'Leif Lindholm (Nuvia address)'
> <leif@nuviainc.com>; Doran, Mark <mark.doran@intel.com>; 'Andrew Fish'
> <afish@apple.com>; Guptha, Soumya K <soumya.k.guptha@intel.com>
> Subject: Re: [edk2-devel] more development process failure [was:
> UefiPayloadPkg: Runtime MMCONF]
> 
> Jiewen,
> 
> On 09/17/20 04:31, Yao, Jiewen wrote:
> > Hi Laszlo, Liming, and Mike
> >
> > I appreciate your effort to setup rule and document this *complex* EDK II
> Development Process.
> >
> > I am thinking if we can have a way (a tool) to mandate these process and
> check if there is any violation. If people makes mistake, he/she knows he/she is
> making mistake and can correct immediately, instead of letting mistake happens
> and getting blame later. In such way, we can prevent issue from happening.
> >
> > We have old maintainer leaving, new maintainers joining. That is the reality.
> We can have training for everyone. But we are still human. There are many bugs
> need to be fixed in the code. How can we expect a perfect process that
> everyone follows strictly without any violation?
> >
> > If we only have few violation, it is OK to stay with it.
> > But if we continuously have violation, we need retrospect to ask, *why*? Why
> there is such a process to cause so many violation?
> > And can we do better? A simpler process? A better tool?
> 
> while I agree that the current process is not really simple, I'd like to
> point out some things:
> 
> - The current complexity exists because we are in a transition period,
> and so we get to deal with both the workflow we're leaving (= the
> mailing list based review) and the system we're adopting (= github).
> This should not last forever. I don't know the exact schedule though.
> 
> - I think that lack of attention to detail (on the human side) takes a
> relatively large chunk of the blame. The process at the moment is not
> simple, but it's exercised every day, every week by some people, so if
> somebody *wants*, they can get it right by following examples. Look at
> recent patch series threads that have been merged, recent BZs that have
> been closed, recent PRs that have been opened and merged.
> 
> It's a fallacy that adopting a 100% github.com-native patch review
> workflow will solve all of these issues. There is no replacement for
> human discipline and attention to detail. In the current process, I
> *regularly* find pull requests (personal builds or maintainer push
> attempts) on github.com that fail CI (or merging due to conflicts) and
> then the submitter never bothers to close or refresh them. I have
> cleaned up (closed) a *multitude* of such PRs.
> 
> > I also feel sorry that Laszlo need check by his eye on every PR and catch the
> violation for us. And I also feel sorry to blame some people who is contributing
> his/her time to help to maintain the code, review the code, check in the code.
> > We both feel frustrated. We are all coming her to enable new features or fix
> bugs to make EDKII better.
> >
> > I would like to ask: Is that technically possible to enhance the CI to catch that
> earlier, as Laszlo point out:
> > 1) Add patch 0 to PR - can we let CI reject empty description PR?
> 
> It won't help.
> 
> See the following bug report:
> 
>   https://bugzilla.tianocore.org/show_bug.cgi?id=2963#c0
> 
> While it is technically not empty (the string in comment#0 has nonzero
> length),  it's practically *devoid of information*.
> 
> People that are annoyed that they are required to write sensible
> summaries for patch sets and bug reports, will do anything and
> everything to wiggle out of that requirement. They will create
> single-sentence PR descriptions, which will allow them to pass the CI
> check. And the community will be *worse off*, because we will have
> complicated our CI logic, but the resultant historical records will be
> just as useless.
> 
> > 2) Send email - can we let CI send email automatically? Or remind us to send
> email?
> 
> github.com *already* sends an email notification when a PR undergoes a
> state change; that is, when it is merged, or else CI fails. The email is
> *already* there, people just have to *act* upon it -- run a local "git
> pull" again, see what the new commit range is, and send a response to
> the original thread.
> 
> > 3) update Bugzilla - can we let CI update Bugzilla automatically? Or remind us
> to update bugzilla?
> 
> Automatically closing tickets is not implemented between github.com and
> Bugzilla. It is implemented within github.com (merging a PR can
> auto-close issue tracker tickets, if you format the commit message
> corectly).
> 
> However, auto-closing is *wrong*. It occurs that multiple patch series
> relate to a single ticket. In such cases, it's possible that 10+ patches
> are merged for a single ticket, and the ticket should *still* not be
> closed, because more patches (for the same ticket) are necessary. Only a
> human can tell when the fix or the feature is considered complete
> (according to their knowledge at that point in time).
> 
> > 4) Unicode char - can we add check in patchchecker, to reject predefined
> format violation?
> 
> There are many-many classes of unicode code points. It's not easy to
> express "accept U+003A for punctuation, but do not accept U+FF1A".
> 
> It's easy to express "accept 7-bit ASCII only", but I think some people
> might take issue with that, because then their names could not be placed
> in subject lines in native form.
> 
> >
> > I know the new tool/CI cannot be built in one day. And we do improvement
> step by step.
> 
> The *real* problem is with the attitude. If a developer cares *only*
> until their patches are merged, then no tooling will *ever* fix this
> issue. People need to start caring about the afterlife of their work.
> When you throw a party, or join one, you stay around for the cleanup
> afterwards, don't you?
> 
> When you call a contractor to fix something in or around the house, do
> you expect them to clean up when they're done, or are you happy cleaning
> after them?
> 
> 
> The exact same bad attitude is the reason that
> 
> - we have botched error paths in programming languages like C,
> 
> - we have programming languages and libraries that attempt (and *fail*)
> to clean up resources on errors, "on behalf" of the programmer -- I'm
> referring to exceptions and destructors in C++, for example.
> 
> Both of these are symptoms that people *refuse* to deal with the
> "boring" aspects of the job.
> 
> 
> Just accept that the party isn't finished until the house and the garden
> are tidied up, and the furniture is restored to original order.
> 
> Laszlo
> 
> 
> 
> 
> 


  reply	other threads:[~2020-09-17  7:31 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-18  8:24 [PATCH v5 0/3] UefiPayloadPkg: Runtime MMCONF Marcello Sylvester Bauer
2020-08-18  8:24 ` [PATCH v5 1/3] UefiPayloadPkg: Store the size of the MMCONF window Marcello Sylvester Bauer
2020-08-18  8:24 ` [PATCH v5 2/3] MdePkg: PciExpressLib support variable size MMCONF Marcello Sylvester Bauer
2020-08-20 10:55   ` Liming Gao
2020-08-18  8:24 ` [PATCH v5 3/3] UefiPayloadPkg: Support variable size MMCONF space Marcello Sylvester Bauer
2020-09-08 22:35   ` [edk2-devel] " Guo Dong
2020-09-16  8:56 ` more development process failure [was: UefiPayloadPkg: Runtime MMCONF] Laszlo Ersek
2020-09-16 17:30   ` [edk2-devel] " Guo Dong
2020-09-16 18:14     ` Laszlo Ersek
2020-09-16 21:51       ` Guo Dong
2020-09-17  5:59         ` Laszlo Ersek
2020-09-17  1:49       ` 回复: " gaoliming
2020-09-17  2:31         ` Yao, Jiewen
2020-09-17  6:31           ` Laszlo Ersek
2020-09-17  7:31             ` Yao, Jiewen [this message]
2020-09-17 10:26               ` Laszlo Ersek
2020-09-18  4:39             ` Ni, Ray
2020-09-18  7:37               ` Andrew Fish
2020-09-26  0:34                 ` Guo Dong
2020-09-27  1:44                   ` 回复: " gaoliming
2020-09-27 17:29                     ` Guo Dong
2020-09-28 17:55                   ` Laszlo Ersek
2020-09-29  4:13                     ` Guo Dong
2020-09-29 11:59                       ` Laszlo Ersek
2020-09-17  5:56         ` 回复: " Laszlo Ersek
2020-09-21  9:57   ` Marcello Sylvester Bauer
2020-09-22  6:45     ` Laszlo Ersek

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=CY4PR11MB1288B90FC46FB96DFA5642B28C3E0@CY4PR11MB1288.namprd11.prod.outlook.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