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 <https://github.com/tianocore/edk2/>. 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 request details present in the blurb. I'll also continue doing reviews on the list only. For other kinds of analyses, I'm very comfortable writing them up in issue tracker items -- once our issue tracker is Bugzilla, operated by Intel (or anyone else with direct, high stakes in edk2). > 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. You could have asked the submitter to push it somewhere :) > Perhaps if we can fix the CRLF thing then applying patches will get a > little easier. It should, yes. > In fact they might even apply now to my converted tree; > I'll go and have a play... If they don't, the only things that need fixing in saved patch emails (for "git am --keep-cr") are the /dev/null hunk headers. sed -r -i 's,^((---|\+\+\+) /dev/null)\r$,\1,' *.eml Thanks Laszlo _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
