On 03/18/16 12:21, David Woodhouse wrote: > On Thu, 2016-03-17 at 21:28 +0000, 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, <https://github.com/tianocore/edk2/pull/71>, 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 tomorrow, we'll lose our comments. So let's not discuss stuff on github.com. --*-- Other than that, it's difficult for me as well to find use cases when #3 (the central staging repo) is unavoidably necessary. Thanks Laszlo _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
