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 huge step > backward. I will also stop analyzing and undoing problems introduced > by others. You you won't :) Because aside from a test to play with them, we'd only actually *do* it if it didn't cause problems in practice. And if it doesn't cause problems in practice then you won't stop helping out. (Also, once we start doing merges properly then history will be preserved and it'll be a *whole* lot easier to work stuff out if it goes wrong. You'll be able to say "you screwed up the merge" because it'll actually be shown correctly. Rather than having to guess what went wrong, as in that OpenSSL example I showed last week.) Anyway, the 'staging proposal' included *personal* github pull requests already as its "method 2". All I was saying that we could *also* use PRs for the long-term 'staging' submissions as well. > 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. FWIW, I am *already* not testing things that are only sent to the mailing list. I wanted to test the TLS support stuff, but it was just too hard to make the patches apply. If it had been in a tree somewhere I could pull from, I'd have actually done it, and I'd have tested that the HTTPS support actually works both with OpenSSL 1.0.2 and 1.1. But it was only on the mailing list, so I didn't. Perhaps if we can fix the CRLF thing then applying patches will get a little easier. In fact they might even apply now to my converted tree; I'll go and have a play... -- 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
