Re: [edk2] EDK2 Staging Proposal 3rd draft, final?

[Kinney, Michael D]

David,

I responded to an earlier email:

==
I think there are several methods we can support for development.

1) Simple bug fixes/features sent directly to edk2-devel for PRs.
2) A larger or more complex bug fix/feature can optionally post a link
   to a branch on personal github fork to help simplify the review process 
   for those reviewers that prefer to use that method.  This type of bug 
   fix or feature is usually owned by a single subject matter expert.
3) A larger or more complex feature that requires design/dev/test by more
   than one subject matter expert.

We already support (1) and (2) today.  Feature branches on edk2-staging are 
intended for (3).

I can think of a couple ways we end up in (3). The first is a feature we know
requires multiple subject matter experts and the request is made to add
to edk2-staging from the beginning.  The second is a PR that is sent to 
edk2-devel, and the community believes it needs more design/dev/test work
that requires cooperation of multiple subject matter experts and the PR 
is re-directed to a feature branch in edk2-staging.
==

Mike

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of David
> Woodhouse
> Sent: Wednesday, March 16, 2016 2:05 PM
> To: Mangefeste, Tony <tony.mangefe...@intel.com>; edk2-devel@lists.01.org 
>  de...@ml01.01.org>
> Subject: Re: [edk2] EDK2 Staging Proposal 3rd draft, final?
> 
> On Wed, 2016-03-16 at 20:17 +, Mangefeste, Tony wrote:
> >
> > Here's the 3rd iteration based on your input.  Please take one last
> > opportunity to review, we'll settle on this if there are no major
> > requests by Friday of this week.
> 
> I still haven't seen an answer to my question of what this buys us,
> over the normal process of having such submissions come via
> contributors' github repositories, with associated pull requests.
> 
> Why invent new processes and not just use the existing tools that are
> basically *designed* for this workflow?
> 
> --
> David WoodhouseOpen Source Technology Centre
> david.woodho...@intel.com  Intel Corporation

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] EDK2 Staging Proposal 3rd draft, final?

[David Woodhouse]

On Fri, 2016-03-18 at 14:24 +0100, Laszlo Ersek wrote:
> Except my own actions. I'm already watching the github issue tracker and
> get emails of the actions of others. No emails about my own actions. It
> makes the email audit trail completely unusable.

That's your personal email notification. If it's set up to mail
everything to the list (even if we create a dummy github user for the
list, and subscribe it to everything), then that won't be an issue,
will it?

Actually, I just went poking about in the settings for repositories. I
see there appears to be a setting to disable merges by merge commit or
by squashing. If you turn those both off on a repository, what happens?
Does that not prevent the merges you were worried about, in either
form?

> I must have missed those discoverability criteria, apologies. In any
> case, I can hardly imagine anything more discoverable than a [PULL] on
> the mailing list.

I think the idea is you should be able to go to e web page and easily
find a list of the stuff that is currently outstanding.

Searching a mailing list for pull requests doesn't really help, because
there's no easy way to see which are merged and which are not.

Patchwork does that kind of thing for some projects, but I don't think
patchwork is the right tool for managing submissions which are intended
to be outstanding for some time, as they undergo refinement and testing
— like the OpenSSL one, for example. There's no way we want that until
the final 1.1 release but we *do* want people to be playing with it.


> I disagree. It is costly. We've had two unintended merges thus far (from
> incorrect command line usage), and they took me personally hours to
> analyze and explain.

I love you, Laszlo, and I *love* the fact that you will take hours to
analyse and explain things. You do it *very* well, and it has been
extremely useful to me and to a lot of other people.

But sometimes perhaps you need to resist the temptation to do it :)

Sometimes, stuff just broke and you just fix it up and move on. Let the
people who *did* it work it out for themselves and that actually forms
a *better* learning experience for them.

And as for committing trivial fixes without review... well, sometimes
it's better to ask for forgiveness than permission. We should never
allow a process to get in the way of getting real work done.

> The fact that these issues were introduced while using the git command
> line supports your point that such problems can occur "anyway". My point
> is that I would like to keep them as rare as possible, and github pull
> requests don't point in that direction.

As noted above, do you want to look in
https://github.com/tianocore/edk2/settings and try turning off the
'Allow merge commits with the merge button' and 'Allow squash merging
with the merge button' options? Or is that only for personal projects?
I don't see it in other github repositories that I have write access
to; only my own.

> If you are volunteering to analyze and undo unintended or incorrectly
> resolved merges from github pull reqs -- I have no further comments then.
> 
> > 
> > And the chances of the CRLF conversion one actually *applying* if
> > someone stupidly hits the button, after *one* more commit hits the
> > tree, are fairly much zero anyway. So you're defending against an
> > impossibility in that case.
> In this specific case, you are right. My point was to prevent building a
> precedent.
> 
> > 
> > I don't know if I should be offended on behalf of the maintainers who
> > are mostly my colleagues, that you have such a low opinion of their
> > capabilities.
> I don't have a low opinion of the capabilities of other edk2
> maintainers. It's a fact that they have just recently started using git.
> It is another fact that I personally mis-click a button (or mistype a
> shortcut) in this or that GUI application occasionally.
> 
> Fixing build breakage (or functional breakage) in the edk2 repository is
> not exactly fun. I find myself analyzing and fixing up bugs from others
> (outside of OvmfPkg/) more and more. Simply because I tend to run into
> them with OVMF (and with Gerd's Jenkins instance) earlier than most
> other people. (I'm not always the first to find out, of course.) I think
> it's natural from me not wanting to ease the introduction of such issues.

Hm, don't we have automatic CI on our pull requests? That's an
oversight... the tools can run builds on various platforms and right
there in the ticket it can show that it doesn't build in certain
configurations. With a big red cross right above the 'merge' button :)

(Which reminds me, I must try to fix things up so that OpenSSL PRs get
tested in something as close to the EDK2 build setup as possible.)

> I'd like to invite other edk2 maintainers to chime in on a github PR
> centered workflow. If the consensus is to use them,

The point here was to have a *look* at them, to make an informed
decision.

>  I'll try to adapt the best I can, although I think it's a 

Re: [edk2] EDK2 Staging Proposal 3rd draft, final?

[Kinney, Michael D]

David,

Yes.  Use of developer github forks is supported.  I had summarized
3 development methods earlier in this thread.

1) PR emails send to edk2-devel.  There is a Wiki page that details process
for developers and maintainer.  

2) Branch on developer owned github fork of edk2.  There have been discussions 
on
how to support a pull request.  This is still under investigation.  In the 
meantime, a link to the branch and pull request can be included in
the PR emails to edk2-devel.

3) Branch on edk2-staging (Specific process being discussed in this thread)

In order to properly capture your feedback, I think a higher level overview
Wiki page on the development methods available needs to be written up with
edk2-staging just being one of the 3 methods (and likely most rare).

I will work with Tony to get Wiki pages updated with all this information.

Will this address your concerns?

Mike

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of David
> Woodhouse
> Sent: Thursday, March 17, 2016 1:39 PM
> To: Kinney, Michael D <michael.d.kin...@intel.com>; Mangefeste, Tony
> <tony.mangefe...@intel.com>; edk2-devel@lists.01.org <edk2-de...@ml01.01.org>
> Subject: Re: [edk2] EDK2 Staging Proposal 3rd draft, final?
> 
> I could have sworn I'd responded to this last night, but it was late,
> and I see no evidence of such in my outbox or on the list. Apologies if
> I'm repeating myself...
> 
> On Thu, 2016-03-17 at 00:15 +, Kinney, Michael D wrote:
> > Jordan asked a similar question about adding a 'staging' directory or 
> > 'staging'
> > branches to the edk2 repo.  If you look at the edk2 repo today, it has 
> > master
> > and the supported UDK* branches.  In the transition from SVN -> GIT, the 
> > old tags
> > were removed and archived.
> >
> > The reason for suggesting a new edk2-staging repo is to keep content that
> > is not ready for products to be clearly separated.  We do not want anyone
> > to pick up any of the staging content and put it into shipping FW images.
> >
> > There was also a minor concern about the size of the edk2 repo, but it
> > was shown that this is not really an issue at this time.
> >
> > So the choice here is a set of staging feature branches in the current
> > edk2 repo with clear communication that none of them are product ready.
> > Or a new edk2-staging repo with feature branches (current proposal).
> 
> This seems to me to be a false dichotomy. You've left out the third
> option, which is just to use the tools as they were designed to be
> used.
> 
> Let people set up their submissions in github repositories of their own
> (which works for groups as well as individuals, and allows loosely-
> formed collaborative groups as well as more formal projects). And you
> can already track their submissions at
> https://github.com/tianocore/edk2/pulls
> 
> > I think a process to create/merge/remove feature branches in the
> > tianocore.org repos is required because we really do not want too
> > many branches.  If the feature being developed is smaller in
> > scope, developer owned github branches or email PRs should be sufficient.
> 
> Developer-owned github repositories are suitable for larger submissions
> too, surely? (With associated discussion on the mailing list which was
> always part of the plan, of course. I'm not suggesting that it should
> necessarily happen *only* in the github PR format.)
> 
> --
> dwmw2

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] EDK2 Staging Proposal 3rd draft, final?

[David Woodhouse]

On Fri, 2016-03-18 at 17:08 +0100, Laszlo Ersek wrote:
> On 03/18/16 14:59, David Woodhouse wrote:
> > Sometimes, stuff just broke and you just fix it up and move on. Let the
> > people who *did* it work it out for themselves and that actually forms
> > a *better* learning experience for them.
>
> Yes, this is how it should work, but if the fix takes an inconveniently
> long time, the problem will likely impede my own work.

If we can agree on a policy of "if it breaks, revert it", then such
things *shouldn't* take an inconveniently long time.

As long as history is preserved correctly and subsequent work hasn't
been rebased on top of the offending commit(s), of course. If you have
a full history, even if merges have happened, then it's not so hard to
take out the offending commits as long as they're recent.

Of course, CI on submissions helps to avoid the problem altogether. To
a certain extent, at least.
   
> > That's an
> > oversight... the tools can run builds on various platforms and right
> > there in the ticket it can show that it doesn't build in certain
> > configurations. With a big red cross right above the 'merge' button :)
>
> I've never heard of this. Would github operate the CI, or would it be
> some external CI, to be integrated with github?

External, triggered by github.

See https://github.com/openssl/openssl/pulls for example.

Each PR has a green tick or a red cross by it, indicating whether the
automatic builds succeeded. Although it's perhaps a bad example because
OpenSSL has a lot of false failures at the moment. But still you get
the idea. Go into one of the failed, and one of the succeeded, PRs and
you'll see what it looks like in the ticket itself as you view it.

> > > Feel free to reopen the pull requests (if you don't have the
> > > credentials, I can reopen them for you).
>
> You didn't respond to this. Is it okay with you if we delay reopening
> your pulls (#70 and #71) until we can disable the merge button (with
> those two settings you discovered)?

That was the intention implicit in the fact that I waited. I may reopen
the CRLF one as soon as the commit *after* my Windows build fix (qv)
goes into the tree, because that will be *guaranteed* not to merge even
if someone does hit the button :)

> I'll probably feel less threatened about github PRs once the merge
> button is disabled, but I would nonetheless like all patches in a pull
> request to be posted to the list, with the pull request details present
> in the blurb.

Yes, I think that's an important part of any process we come up with
even when it includes github PRs. It doesn't *bypass* the mailing list.

> > Perhaps if we can fix the CRLF thing then applying patches will get a
> > little easier.

> It should, yes.

FWIW this seems to be working. I can do checkouts on Windows and if I
have core.autocrlf set in my git config then I get CRLF line endings in
all the checked out files just as before. And if I *don't* have that
set, then I get LF but it works fine anywy.,

With core.autocrlf set, just pulling the commit which changes the
*internal* representation from CRLF to LF basically does nothing to the
checked-out files since they remain CRLF.

I haven't worked out how to *automatically* set the core.autocrlf for
the repository, similar to the way that .gitattributes works. But given
that things work OK even without it, I think that's fine.

I want to redo all my testing after that VS2008 build fix has landed,
so I'm not perturbing all my tests. And obviously let some other people
test it. But I'm fairly happy with it.

-- 
David WoodhouseOpen Source Technology Centre
david.woodho...@intel.com  Intel Corporation



smime.p7s
Description: S/MIME cryptographic signature
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] EDK2 Staging Proposal 3rd draft, final?

[Laszlo Ersek]

On 03/18/16 13:43, David Woodhouse wrote:
> On Fri, 2016-03-18 at 13:30 +0100, Laszlo Ersek wrote:
>>
>> Our workflow should not be centered on github pull requests in any case,
>> so I don't see the point in testing them out. 
> 
> Well, thanks for destroying the test I spent this morning setting up,
> because you don't believe it would have useful results.

I feel very bad about it. My main reason was that I found it too risky,
setting a precedent.

> I happen to disagree about using github pull requests. Every change
> there triggers an email,

Except my own actions. I'm already watching the github issue tracker and
get emails of the actions of others. No emails about my own actions. It
makes the email audit trail completely unusable.

> and we can set things up so that email is also
> copied to the mailing list. So it's just as well-archived as the other
> list traffic, and we don't have to worry about github.com disappearing
> in a puff of smoke any more than we already did.
> 
>> workflow centered on pull requests that are sent to the mailing list
>> should be tested out, and I'd like to work with you on that.
> 
> That works for me, but doesn't seem to cover the criteria that Michael
> set out for discoverability.

I must have missed those discoverability criteria, apologies. In any
case, I can hardly imagine anything more discoverable than a [PULL] on
the mailing list.

>> I understand that the PRs you created on github explicitly stated they
>> weren't to be merged. It doesn't matter. One misclick from an edk2
>> maintainer who happens to be paying a bit less attention, and poof, we
>> have mess in the git history.
> 
> Which is fairly much true of *any* work, right? Any of those people
> could 'accidentally' push stuff from their working tree out to the main
> repository, at any time. We either revert it with a new commit, or we
> quickly undo the commit and rewind the contents of the tree before too
> many people update from it. It's hardly the end of the world.

I disagree. It is costly. We've had two unintended merges thus far (from
incorrect command line usage), and they took me personally hours to
analyze and explain.

Even without github pull requests, we frequently see build breakage too.
Fixing them up is anything but fast. The fixup patch has to be mailed
out, reviewed, committed. When the build fixup is trivial, I
occasionally take the liberty and commit it without review (and them I'm
super worried if I'll be called out for committing without review).

The fact that these issues were introduced while using the git command
line supports your point that such problems can occur "anyway". My point
is that I would like to keep them as rare as possible, and github pull
requests don't point in that direction.

If you are volunteering to analyze and undo unintended or incorrectly
resolved merges from github pull reqs -- I have no further comments then.

> And the chances of the CRLF conversion one actually *applying* if
> someone stupidly hits the button, after *one* more commit hits the
> tree, are fairly much zero anyway. So you're defending against an
> impossibility in that case.

In this specific case, you are right. My point was to prevent building a
precedent.

> I don't know if I should be offended on behalf of the maintainers who
> are mostly my colleagues, that you have such a low opinion of their
> capabilities.

I don't have a low opinion of the capabilities of other edk2
maintainers. It's a fact that they have just recently started using git.
It is another fact that I personally mis-click a button (or mistype a
shortcut) in this or that GUI application occasionally.

Fixing build breakage (or functional breakage) in the edk2 repository is
not exactly fun. I find myself analyzing and fixing up bugs from others
(outside of OvmfPkg/) more and more. Simply because I tend to run into
them with OVMF (and with Gerd's Jenkins instance) earlier than most
other people. (I'm not always the first to find out, of course.) I think
it's natural from me not wanting to ease the introduction of such issues.

I'd like to invite other edk2 maintainers to chime in on a github PR
centered workflow. If the consensus is to use them, I'll try to adapt
the best I can, although I think it's a huge step backward. I will also
stop analyzing and undoing problems introduced by others. I hope those
in favor of github PRs will step up.

Feel free to reopen the pull requests (if you don't have the
credentials, I can reopen them for you). I'm not offering to help
testing them out any longer; I disagree with the github workflow and
won't be supporting it until I'm forced to, by maintainer consensus.

Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] EDK2 Staging Proposal 3rd draft, final?

[Laszlo Ersek]

On 03/18/16 12:21, David Woodhouse wrote:
> On Thu, 2016-03-17 at 21:28 +, Kinney, Michael D wrote:
>>
>> Yes.  Use of developer github forks is supported.  I had summarized
>> 3 development methods earlier in this thread.
>>
>> 1) PR emails send to edk2-devel.  There is a Wiki page that details process
>> for developers and maintainer.  
>>
>> 2) Branch on developer owned github fork of edk2.  There have been 
>> discussions on
>> how to support a pull request.  This is still under investigation.  In the 
>> meantime, a link to the branch and pull request can be included in
>> the PR emails to edk2-devel.
>>
>> 3) Branch on edk2-staging (Specific process being discussed in this thread)
> 
> Let's *test* my assertion that github PRs should be sufficient even to
> cover #3.
> 
> They're discoverable — at https://github.com/tianocore/edk2/pulls you
> can see that I've just filed a couple of tickets. I believe they
> satisfies all the other criteria you've mentioned, but let's have an
> experiment and find out.
> 
> I think these two test cases are precisely the kind of submission which
> want to be handled via 'staging', aren't they?
> 
> The first — https://github.com/tianocore/edk2/pull/70 — is the update
> to build against OpenSSL 1.1. I've thrown in a "line note" on the code,
> so you can see how that works as well as the general discussion in the
> ticket.
> 
> The second — https://github.com/tianocore/edk2/pull/71 — is a test of
> converting the internal representation of all files to use git's native
> LF line endings. This should mean that the tools can do the right thing
> and check out the working files according to the norms of the platform
> — you'll get CRLF on Windows, and LF on Linux. It would be good to test
> that, iron out any problems and get it fixed. But again, definitely
> something that wants staging for review and testing first.

Thanks for the work. I'm willing to help you test these.

I'm going to close the github pull requests now. We have a quite large
number of maintainers, and everyone has the credentials to click that
button in the github pull requests. If someone clicks it out of
oversight, that won't lead to happiness. (Your pull requests target
"tianocore:master".)

Instead, please post the pull request to the development mailing list.
That is discoverable too, and has a much lower chance of (a) someone
pulling it into his local clone accidentally, and then (b) pushing the
merge commit to the central repo accidentally. The so-called "ease of
merging" that github offers is a minus for us, not a plus.

The pull request posted to the list should look like this: it should be
a formatted patch series, where the subject prefix is PULL, not PATCH.
The blurb (00/nn) message should contain the output of the
"git-request-pull" command, plus any other comments you might want to add.

Posting the pull request to the mailing list like this allows our
workflow to remain independent of github. GitHub should only remain a
repository hosting site for us. Maybe a wiki too.

(The github issue tracker is already too much, but Tony has been working
on setting up Bugzilla @ Intel, so I hope in the mid term we can move
off of the github issue tracker.)

The discussion of your pull requests should happen on the list, not in
the github pull request ticket. Once we have an independent Bugzilla
instance, one that supports perfect email integration and mass data
archival / export, then we can also discuss technical stuff in Bugzilla
entries at depth.

Now, specifically for the CRLF conversion, I expect the patches to be
huge, and also mechanic, so posting those to the mailing list is
probably not useful. In this case just the blurb should suffice.

The OpenSSL 1.1 support pull is perfectly suitable for patch-wise
posting (as a PULL).

(
Side point: for the CRLF conversion, I repeated your command in a bit
different form:

$ git ls-tree -r --full-tree --name-only master: \
  | file --files-from - \
  | grep CRLF \
  | cut -f1 -d: \
  | xargs cat \
  | wc -l

The intent is to count the number of lines in all those files --
presumably all lines have to be converted. The result is 4,383,397
lines. Okay. (For 14,108 files.)

However, looking at the github pull request,
, the line count in the
cumulative diffstat looks wrong. The number of changed files seems okay
(14,105), but the diffstat says "20,557 lines changed". That's a
fraction of what I expect to be changed, and it doesn't really increase
my trust in github.
)

--*--

github.com is a for-profit company that doesn't seem to have any
official relationship with edk2 contributors. I trust them to host the
repositories (the central one and the personal ones). If something
breaks, we can immediately move those elsewhere. Same for the wiki.

The same is not true of the discussions led in pull request items or
other issue tracker items; github.com practically owns those. If they
lock down their issue tracker 

Re: [edk2] EDK2 Staging Proposal 3rd draft, final?

[David Woodhouse]

I could have sworn I'd responded to this last night, but it was late,
and I see no evidence of such in my outbox or on the list. Apologies if
I'm repeating myself...

On Thu, 2016-03-17 at 00:15 +, Kinney, Michael D wrote:
> Jordan asked a similar question about adding a 'staging' directory or 
> 'staging'
> branches to the edk2 repo.  If you look at the edk2 repo today, it has master 
> and the supported UDK* branches.  In the transition from SVN -> GIT, the old 
> tags
> were removed and archived.
> 
> The reason for suggesting a new edk2-staging repo is to keep content that
> is not ready for products to be clearly separated.  We do not want anyone
> to pick up any of the staging content and put it into shipping FW images.
> 
> There was also a minor concern about the size of the edk2 repo, but it
> was shown that this is not really an issue at this time.
> 
> So the choice here is a set of staging feature branches in the current 
> edk2 repo with clear communication that none of them are product ready.  
> Or a new edk2-staging repo with feature branches (current proposal).

This seems to me to be a false dichotomy. You've left out the third
option, which is just to use the tools as they were designed to be
used.

Let people set up their submissions in github repositories of their own
(which works for groups as well as individuals, and allows loosely-
formed collaborative groups as well as more formal projects). And you
can already track their submissions at
https://github.com/tianocore/edk2/pulls

> I think a process to create/merge/remove feature branches in the 
> tianocore.org repos is required because we really do not want too 
> many branches.  If the feature being developed is smaller in 
> scope, developer owned github branches or email PRs should be sufficient.

Developer-owned github repositories are suitable for larger submissions
too, surely? (With associated discussion on the mailing list which was
always part of the plan, of course. I'm not suggesting that it should
necessarily happen *only* in the github PR format.)

-- 
dwmw2



smime.p7s
Description: S/MIME cryptographic signature
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] EDK2 Staging Proposal 3rd draft, final?

[Kinney, Michael D]

David,

Jordan asked a similar question about adding a 'staging' directory or 'staging'
branches to the edk2 repo.  If you look at the edk2 repo today, it has master 
and the supported UDK* branches.  In the transition from SVN -> GIT, the old 
tags
were removed and archived.

The reason for suggesting a new edk2-staging repo is to keep content that
is not ready for products to be clearly separated.  We do not want anyone
to pick up any of the staging content and put it into shipping FW images.

There was also a minor concern about the size of the edk2 repo, but it
was shown that this is not really an issue at this time.

So the choice here is a set of staging feature branches in the current 
edk2 repo with clear communication that none of them are product ready.  
Or a new edk2-staging repo with feature branches (current proposal).
 
I think a process to create/merge/remove feature branches in the 
tianocore.org repos is required because we really do not want too 
many branches.  If the feature being developed is smaller in 
scope, developer owned github branches or email PRs should be sufficient.

Thanks,

Mike


> -Original Message-
> From: David Woodhouse [mailto:dw...@infradead.org]
> Sent: Wednesday, March 16, 2016 3:23 PM
> To: Kinney, Michael D <michael.d.kin...@intel.com>; Mangefeste, Tony
> <tony.mangefe...@intel.com>; edk2-devel@lists.01.org <edk2-de...@ml01.01.org>
> Subject: Re: [edk2] EDK2 Staging Proposal 3rd draft, final?
> 
> On Wed, 2016-03-16 at 22:05 +, Kinney, Michael D wrote:
> >
> > I responded to an earlier email:
> 
> Ah, apologies — it wasn't clear this was intended as an answer to that
> question. This is one of those examples with the common email behaviour
> of placing the response below a carefully-selected citation would have
> helped a great deal, FWIW.
> 
> > I think there are several methods we can support for development.
> >
> > 1) Simple bug fixes/features sent directly to edk2-devel for PRs.
> > 2) A larger or more complex bug fix/feature can optionally post a link
> >    to a branch on personal github fork to help simplify the review process
> >    for those reviewers that prefer to use that method.  This type of bug
> >    fix or feature is usually owned by a single subject matter expert.
> > 3) A larger or more complex feature that requires design/dev/test by more
> >    than one subject matter expert.
> >
> > We already support (1) and (2) today.  Feature branches on edk2-staging are
> > intended for (3).
> 
> So the answer would be that it allows more than one person to
> collaborate on a submission? I'm not sure we need a whole new process
> document just for that, do we? It's not as if it's hard for people to
> share access to their repositories, or just pull from each other's.
> 
> ==
> 
> On Wed, 2016-03-16 at 15:15 -0700, Andrew Fish wrote:
> > I think David's point is any github repository can be used for this,
> > if I understand David's point?
> 
> Precisely so.
> 
> > So the only value to branches is an official place for them to live?
> > Thus a way they can be discovered.
> 
> Can't they already be discovered at
> https://github.com/tianocore/edk2/pulls ?
> 
> This is the normal workflow. People (individuals or otherwise) put
> together some work, discussion happens (and is archived and
> discoverable) in a pull request. New versions of the tree can be
> submitted to the PR at any time.
> 
> We don't need to invent anything new and different, just to allow more
> than one person to work together on a submission.
> 
> --
> David WoodhouseOpen Source Technology Centre
> david.woodho...@intel.com  Intel Corporation

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] EDK2 Staging Proposal 3rd draft, final?

[David Woodhouse]

On Wed, 2016-03-16 at 20:17 +, Mangefeste, Tony wrote:
> 
> Here's the 3rd iteration based on your input.  Please take one last
> opportunity to review, we'll settle on this if there are no major
> requests by Friday of this week.

I still haven't seen an answer to my question of what this buys us,
over the normal process of having such submissions come via
contributors' github repositories, with associated pull requests.

Why invent new processes and not just use the existing tools that are
basically *designed* for this workflow?

-- 
David WoodhouseOpen Source Technology Centre
david.woodho...@intel.com  Intel Corporation



smime.p7s
Description: S/MIME cryptographic signature
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] EDK2 Staging Proposal 3rd draft, final?

[Laszlo Ersek]

On 03/18/16 14:59, David Woodhouse wrote:

> That's your personal email notification. If it's set up to mail
> everything to the list (even if we create a dummy github user for the
> list, and subscribe it to everything), then that won't be an issue,
> will it?

Perhaps. I've never tried.

> Actually, I just went poking about in the settings for repositories. I
> see there appears to be a setting to disable merges by merge commit or
> by squashing. If you turn those both off on a repository, what happens?
> Does that not prevent the merges you were worried about, in either
> form?

We had discussed disabling merges with Jordan. If I remember correctly,
it didn't seem possible.

I think I may not have the permissions to manage such repository
settings for edk2... Right, I do have a Settings link in my own repo
view, but none at . In my own repo,
I see

- Allow merge commits with the merge button
- Allow squash merging with the merge button

Disabling these seems appropriate. Jordan, do you recall these settings
from the last time you looked?

... Interestingly, after I disabled the above settings in my own repo,
the settings are now entirely gone, without the possibility to reenable
them. I don't understand. :/

>> I must have missed those discoverability criteria, apologies. In any
>> case, I can hardly imagine anything more discoverable than a [PULL] on
>> the mailing list.
> 
> I think the idea is you should be able to go to e web page and easily
> find a list of the stuff that is currently outstanding.
> 
> Searching a mailing list for pull requests doesn't really help, because
> there's no easy way to see which are merged and which are not.

Ah. I've never had a problem with marking & tracking longer term items
in my numerous email folders, including outstanding series from others.
Anyway, I agree that some explicit tracking for pending pull reqs could
be helpful. (Could be tracked by Bugzilla too, if absolutely necessary,
although it would need a bit more manual work.)

> Patchwork does that kind of thing for some projects, but I don't think
> patchwork is the right tool for managing submissions which are intended
> to be outstanding for some time, as they undergo refinement and testing
> — like the OpenSSL one, for example. There's no way we want that until
> the final 1.1 release but we *do* want people to be playing with it.

I think if it is a large feature that might need several versions
(several pull requests), then it's best to track the feature itself with
an issue tracker item, not the pull request for version N. When I have a
bug or RFE open and I'm sending version N out, I just add a comment and
a link (into the mailing list archive) about version N.

> Sometimes, stuff just broke and you just fix it up and move on. Let the
> people who *did* it work it out for themselves and that actually forms
> a *better* learning experience for them.

Yes, this is how it should work, but if the fix takes an inconveniently
long time, the problem will likely impede my own work.

> And as for committing trivial fixes without review... well, sometimes
> it's better to ask for forgiveness than permission.

Yes, I occasionally find the courage to do this, but I'm still worried
afterwards.

> As noted above, do you want to look in
> https://github.com/tianocore/edk2/settings and try turning off the
> 'Allow merge commits with the merge button' and 'Allow squash merging
> with the merge button' options? Or is that only for personal projects?
> I don't see it in other github repositories that I have write access
> to; only my own.

Those settings might only be available to the account holder who created
the repository. If they could be cleared, that would still my worries.

> Hm, don't we have automatic CI on our pull requests?

We don't have any kind of official CI.

Gerd has been running (on his own dime, I shall add) a Jenkins instance
that regularly builds OVMF and ArmVirtPkg with various settings.

> That's an
> oversight... the tools can run builds on various platforms and right
> there in the ticket it can show that it doesn't build in certain
> configurations. With a big red cross right above the 'merge' button :)

I've never heard of this. Would github operate the CI, or would it be
some external CI, to be integrated with github?

>> Feel free to reopen the pull requests (if you don't have the
>> credentials, I can reopen them for you).

You didn't respond to this. Is it okay with you if we delay reopening
your pulls (#70 and #71) until we can disable the merge button (with
those two settings you discovered)?

> I'm not offering to help
>> testing them out any longer; I disagree with the github workflow and
>> won't be supporting it until I'm forced to, by maintainer consensus.

I'll probably feel less threatened about github PRs once the merge
button is disabled, but I would nonetheless like all patches in a pull
request to be posted to the list, with the pull 

Re: [edk2] EDK2 Staging Proposal 3rd draft, final?

[David Woodhouse]

On Fri, 2016-03-18 at 13:18 +0100, Laszlo Ersek wrote:
> 
> Thanks for the work. I'm willing to help you test these.
> 
> I'm going to close the github pull requests now.

Please don't. The point is to look at the workflow that the github PR
tickets allow, not to actually merge the code — which is why they both
explicitly state that they aren't to be merged.

-- 
dwmw2



smime.p7s
Description: S/MIME cryptographic signature
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] EDK2 Staging Proposal 3rd draft, final?

[Laszlo Ersek]

On 03/18/16 13:25, David Woodhouse wrote:
> On Fri, 2016-03-18 at 13:18 +0100, Laszlo Ersek wrote:
>>
>> Thanks for the work. I'm willing to help you test these.
>>
>> I'm going to close the github pull requests now.
> 
> Please don't. The point is to look at the workflow that the github PR
> tickets allow, not to actually merge the code — which is why they both
> explicitly state that they aren't to be merged.

Our workflow should not be centered on github pull requests in any case,
so I don't see the point in testing them out. I perfectly agree that a
workflow centered on pull requests that are sent to the mailing list
should be tested out, and I'd like to work with you on that.

I understand that the PRs you created on github explicitly stated they
weren't to be merged. It doesn't matter. One misclick from an edk2
maintainer who happens to be paying a bit less attention, and poof, we
have mess in the git history.

I apologize for annoying you, but I've closed them right after sending
my previous email. :(

Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] EDK2 Staging Proposal 3rd draft, final?

[David Woodhouse]

On Thu, 2016-03-17 at 21:28 +, Kinney, Michael D wrote:
> Yes.  Use of developer github forks is supported.  I had summarized
> 3 development methods earlier in this thread.
> 
> 1) PR emails send to edk2-devel.  There is a Wiki page that details process
> for developers and maintainer.  
> 
> 2) Branch on developer owned github fork of edk2.  There have been 
> discussions on
> how to support a pull request.  This is still under investigation.  

I'm glad to see this. Note that the 'backporting' into Subversion (as
Jordan mentioned) shouldn't be an issue; you can always *choose* a
linear path through the DAG of commits, so a merge such as

  A → B → C → D
   ↘           ↘
    B' →  C' →  E

...can *either* be represented as A → B → C → D → E in Subversion, or
perhaps as A → B' → C' → E depending on which side was actually *seen*
in the EDK2 repository in the interim.

But it certainly shouldn't be a blocker for allowing merge commits, and
I can't think of anything *else* that would be.

> 3) Branch on edk2-staging (Specific process being discussed in this thread)
> 
> In order to properly capture your feedback, I think a higher level overview
> Wiki page on the development methods available needs to be written up with
> edk2-staging just being one of the 3 methods (and likely most rare).
> 
> I will work with Tony to get Wiki pages updated with all this information.
> 
> Will this address your concerns?

It still hasn't really answered the question of why we need a whole new
tree, process and associated documentation for this third and 'most
rare' method of merging contributions, when the two existing standard
methods should happily cover all use cases.

It just seems entirely gratuitous.

-- 
dwmw2



smime.p7s
Description: S/MIME cryptographic signature
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] EDK2 Staging Proposal 3rd draft, final?

[Andrew Fish]

> On Mar 16, 2016, at 3:05 PM, Kinney, Michael D <michael.d.kin...@intel.com> 
> wrote:
> 
> David,
> 
> I responded to an earlier email:
> 
> ==
> I think there are several methods we can support for development.
> 
> 1) Simple bug fixes/features sent directly to edk2-devel for PRs.
> 2) A larger or more complex bug fix/feature can optionally post a link
>   to a branch on personal github fork to help simplify the review process 
>   for those reviewers that prefer to use that method.  This type of bug 
>   fix or feature is usually owned by a single subject matter expert.
> 3) A larger or more complex feature that requires design/dev/test by more
>   than one subject matter expert.
> 
> We already support (1) and (2) today.  Feature branches on edk2-staging are 
> intended for (3).
> 
> I can think of a couple ways we end up in (3). The first is a feature we know
> requires multiple subject matter experts and the request is made to add
> to edk2-staging from the beginning.  The second is a PR that is sent to 
> edk2-devel, and the community believes it needs more design/dev/test work
> that requires cooperation of multiple subject matter experts and the PR 
> is re-directed to a feature branch in edk2-staging.

I think David's point is any github repository can be used for this, if I 
understand David's point?

So the only value to branches is an official place for them to live? Thus a way 
they can be discovered.

Thanks,

Andrew Fish

> ==
> 
> Mike
> 
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of David
>> Woodhouse
>> Sent: Wednesday, March 16, 2016 2:05 PM
>> To: Mangefeste, Tony <tony.mangefe...@intel.com>; edk2-devel@lists.01.org 
>> > de...@ml01.01.org>
>> Subject: Re: [edk2] EDK2 Staging Proposal 3rd draft, final?
>> 
>> On Wed, 2016-03-16 at 20:17 +, Mangefeste, Tony wrote:
>>> 
>>> Here's the 3rd iteration based on your input.  Please take one last
>>> opportunity to review, we'll settle on this if there are no major
>>> requests by Friday of this week.
>> 
>> I still haven't seen an answer to my question of what this buys us,
>> over the normal process of having such submissions come via
>> contributors' github repositories, with associated pull requests.
>> 
>> Why invent new processes and not just use the existing tools that are
>> basically *designed* for this workflow?
>> 
>> --
>> David WoodhouseOpen Source Technology Centre
>> david.woodho...@intel.com  Intel Corporation
> 
> ___
> 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

Re: [edk2] EDK2 Staging Proposal 3rd draft, final?

[David Woodhouse]

On Fri, 2016-03-18 at 13:30 +0100, Laszlo Ersek wrote:
> 
> Our workflow should not be centered on github pull requests in any case,
> so I don't see the point in testing them out. 

Well, thanks for destroying the test I spent this morning setting up,
because you don't believe it would have useful results.

I happen to disagree about using github pull requests. Every change
there triggers an email, and we can set things up so that email is also
copied to the mailing list. So it's just as well-archived as the other
list traffic, and we don't have to worry about github.com disappearing
in a puff of smoke any more than we already did.

> workflow centered on pull requests that are sent to the mailing list
> should be tested out, and I'd like to work with you on that.

That works for me, but doesn't seem to cover the criteria that Michael
set out for discoverability.

> I understand that the PRs you created on github explicitly stated they
> weren't to be merged. It doesn't matter. One misclick from an edk2
> maintainer who happens to be paying a bit less attention, and poof, we
> have mess in the git history.

Which is fairly much true of *any* work, right? Any of those people
could 'accidentally' push stuff from their working tree out to the main
repository, at any time. We either revert it with a new commit, or we
quickly undo the commit and rewind the contents of the tree before too
many people update from it. It's hardly the end of the world.

And the chances of the CRLF conversion one actually *applying* if
someone stupidly hits the button, after *one* more commit hits the
tree, are fairly much zero anyway. So you're defending against an
impossibility in that case.

I don't know if I should be offended on behalf of the maintainers who
are mostly my colleagues, that you have such a low opinion of their
capabilities.

-- 
dwmw2



smime.p7s
Description: S/MIME cryptographic signature
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] EDK2 Staging Proposal 3rd draft, final?

[Mangefeste, Tony]

Folks,

Here's the 3rd iteration based on your input.  Please take one last opportunity 
to review, we'll settle on this if there are no major requests by Friday of 
this week.

Problem statement
=
Need place on tianocore.org where new features that are not ready for product 
integration can be checked in for evaluation by the EDK II community prior to 
adding to the edk2 trunk.  This serves several purposes:

* Encourage source code to be shared earlier in the development process
* Allow source code to be shared that does not yet meet all edk2 required 
quality criteria
* Allow source code to be shared so the EDK II community can help finish and 
validate new features
* Provide a location to hold new features until they are deemed ready for 
product integration
* Provide a location to hold new features until there is a natural point in 
edk2 release cycle to fully validate the new feature.
* Not intended to be used for bug fixes.
* Not intended to be used for small, simple, or low risk features.

Proposal

1) Create a new repo called edk2-staging
a) edk2-staging is a fork of edk
b) edk2-staging/master tracks edk2/master

2) All edk2-staging discussions use the existing edk2-devel mailing list for 
design/patch/test.
Use the following style for discussion of a specific feature branch in 
edk2-staging repo.

[staging/branch]: Subject

3) All commits to edk2-staging must follow same edk2 rules (e.g. Tiano 
Contributor's Agreement)

4) Process to add a new feature to edk2-staging
a) Request to create a new edk2-staging feature branch sent to 
edk2-devel
Request should include feature summary, owners, timeline, and quality 
criteria to add to edk2.
If Request is a platform or package specific feature, pre-approval for 
edk2 trunk promotion may be requested here.
b) Branch request and branch name must be approved by stewards
c) Branch maintainer creates edk2-staging feature branch
d) Branch maintainer creates Readme.MD in root of feature branch with 
summary, owners, timeline, links to related materials.
e) Branch maintainer is responsible for making sure feature is 
frequently synced to edk2/master

 NOTE: Feature branch may initially use a stable edk2 tag.  As feature 
stabilizes, syncs to edk2/master can begin.

5) Process to update sources in feature branch
a) Patch email send to edk2-devel
b) Commit message subject format

[staging/branch PATCH]: Package/Module: Subject

c) Use same edk2-devel review process 
d) If pass review, then maintainer commits change to edk2-staging 
feature branch

NOTE: win32 binaries are not automatically generated if a feature 
branch includes BaseTools source changes.

6) Process to promote an edk2-staging branch to edk2 trunk
a) Request sent to edk2-devel that describes the feature, design, 
testing, etc.
b) Stewards evaluate request and determine if the feature meets edk2 
criteria.
c) If approved, use edk2 patch review/commit process on edk2-devel 
mailing list.
The specific process used to add the feature to edk2 is at the 
discretion of the person doing the merge.
Some examples are use of 'git rebase' and 'git pull'.  A merge commit 
must always contain the final 'Reviewed-by:' tags.
d) Remove feature branch from edk2-staging and archive at 
https://github.com/tianocore/edk2-archive.

7) Process to remove an edk2-staging branch
a) Stewards periodically review of feature branches in edk2-staging 
(once a quarter?)
b) If no activity for extended period of time and feature is no longer 
deemed a candidate for edk2 then stewards send email to edk2-devel to request 
deletion of feature branch.  
c) If no objections from EDK II community, then feature branch is 
deleted and archived at https://github.com/tianocore/edk2-archive.

8) Process to evaluate a feature in edk2-staging
a) Clone edk2
b) Create local branch with optional platform packages
c) Build platform in local branch and verify it is stable
d) Clone edk2-staging/[branch name]
e) Create local branch with optional platform packages
f) Build platform in local branch and evaluate new feature



BRs,
Tony

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] EDK2 Staging Proposal 3rd draft, final?

[David Woodhouse]

On Thu, 2016-03-17 at 21:28 +, Kinney, Michael D wrote:
> 
> Yes.  Use of developer github forks is supported.  I had summarized
> 3 development methods earlier in this thread.
> 
> 1) PR emails send to edk2-devel.  There is a Wiki page that details process
> for developers and maintainer.  
> 
> 2) Branch on developer owned github fork of edk2.  There have been 
> discussions on
> how to support a pull request.  This is still under investigation.  In the 
> meantime, a link to the branch and pull request can be included in
> the PR emails to edk2-devel.
> 
> 3) Branch on edk2-staging (Specific process being discussed in this thread)

Let's *test* my assertion that github PRs should be sufficient even to
cover #3.

They're discoverable — at https://github.com/tianocore/edk2/pulls you
can see that I've just filed a couple of tickets. I believe they
satisfies all the other criteria you've mentioned, but let's have an
experiment and find out.

I think these two test cases are precisely the kind of submission which
want to be handled via 'staging', aren't they?

The first — https://github.com/tianocore/edk2/pull/70 — is the update
to build against OpenSSL 1.1. I've thrown in a "line note" on the code,
so you can see how that works as well as the general discussion in the
ticket.

The second — https://github.com/tianocore/edk2/pull/71 — is a test of
converting the internal representation of all files to use git's native
LF line endings. This should mean that the tools can do the right thing
and check out the working files according to the norms of the platform
— you'll get CRLF on Windows, and LF on Linux. It would be good to test
that, iron out any problems and get it fixed. But again, definitely
something that wants staging for review and testing first.

-- 
David WoodhouseOpen Source Technology Centre
david.woodho...@intel.com  Intel Corporation



smime.p7s
Description: S/MIME cryptographic signature
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel