-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 On Sat, Dec 10, 2016 at 12:05:07AM -0500, Jean-Philippe Ouellet wrote: > (Re-ordered the quoted email for more readable reply) > > On Fri, Dec 9, 2016 at 11:06 PM, Marek Marczykowski-Górecki > <marma...@invisiblethingslab.com> wrote: > > Depending only on github repo permissions is against the rule of not > > trusting the infrastructure. > > I definitely agree, and on the "here's what we actually download and > compile" there is no substitute for consistent signature verification. > However, for the purpose of tracking what work is to be done, we > already trust github very much.
Yes, that's right... > > As we don't trust the external infrastructure, everything downloaded > > from the network have signatures checked - this include checking git > > tags on git fetch/pull. And I think exactly this mechanism can be used > > to mark reviewed code. So, a community member could push a code to > > qubesos-contrib/... repo, then wait for it being reviewed - and when got > > reviewed, will get tagged by qubes core team. This will allow (and > > actually will trigger) build process. > > The purpose of the github PRs here would be to improve the workflow of > a maintainer telling the world "Hey, here's a set of complete and > locally-tested changes, please review." with two goals: > 1) Help ensure things get reviewed in a timely fashion and don't get > forgotten about. > 2) Provide a clear default way to provide feedback in the case of > unacceptable changes. Indeed, especially the second point makes very much sense. I think we should require such PR to be fast-forward, to avoid conflict resolution (or unintended interactions of changes, even without conflicts). > This is a much more familiar workflow to people than a push of a > signed tag meaning "please review this, and push another signed tag". > What if these changes are unacceptable when reviewed? Do we open an > issue? AFAICT PRs are just issues with additional user interface to > refer to specific changes in exactly the manner we would need to > poorly emulate with issues. > > The actual reviewing should happen on a developers local machine (not > github UI), to: > 1) verify that the changes being reviewed are indeed what the > maintainer proposes (by verifying the maintainer's tag) > 2) verify that all commits since the last review have indeed been > reviewed (as determined by the reviewer's own last signed tag) > > Then it's a simple fast-forward of master pushing to close the PR. No > additional trust in github besides essentially for to-do list > management (which we already do). Ok, I agree. But I still think another organisation makes sense. Mostly for two reasons: - badge for the author of such tool - different (less strict) rules for repository naming scheme > IMO something like this should already be the workflow for reviewing > PRs to core (non -contrib) code. (Perhaps it is, idk.) It is actually the case for non-trivial changes, take a look for example here (and also merged ones): https://github.com/QubesOS/qubes-core-admin/pulls Github probably show "merged by marmarek" in all the cases, but for core3-devel branch (upcoming Qubes 4.0) the actual workflow is: 1. Pull request against qubesos/qubes-core-admin:core3-devel 2. Wojtek review the changes and push to woju/qubes-core-admin:core3-devel-staging (together with signed tag of course) 3. Travis CI run automated tests and if succeed, update his core3-devel to what is in core3-devel-staging (this is fast-forward, so signed tag still matches the code) 4. Another script monitor what is in woju/qubes-core-admin:core3-devel and update qubesos/qubes-core-admin:core3-devel accordingly (only if the change is fast-forward and properly signed of course). This is the workflow we started in this repository and probably will implement in others too (especially using PR, then pushing to -staging to always go through automated tests). Another practice used by many open-source projects it to route all the patches through the mailing list. But I find github more convenient, plus it preserve signed tags + commits... Previous workflow was to push to personal repository first (in many cases directly to master branch) and push to qubesos org after changes being reviewed. This is why historically code in my github account was fresher than in qubesos org. - -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJYTA0xAAoJENuP0xzK19csYxgH/imqsBItDjr67MCC1VWwpCTc Q25dw/Vd24L1MmxI9fCsDpTMg8AfHFnXYFRoDiHrCcEKXQwzv2JzH4BfWqKGqZwH Dm94LKcMSQlUAUkP5oQf7E6um5HvL7BIdtKI015IXrsaDZjhIxqx25lHTkOuFBYH cUY6YcjC4BopaVX6pNWdCKpdw/rGWzdRD70kQHl/+mDR5I7jiehiKQ6qkTCD+7TY lDKZlZNzxkG82Is8GYVJShSzzvTkN1wiqgEGuGFhr9FHVQ8GjjaEhM/4xQrGUFgc 8lR4gzRBrIOAYEoaiJ7J/bASCwOCEtdKOlq80tPNjH7mvWeTyvInyDTM2h5LMG8= =K0P+ -----END PGP SIGNATURE----- -- You received this message because you are subscribed to the Google Groups "qubes-devel" group. To unsubscribe from this group and stop receiving emails from it, send an email to qubes-devel+unsubscr...@googlegroups.com. To post to this group, send email to qubes-devel@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/qubes-devel/20161210141201.GL16264%40mail-itl. For more options, visit https://groups.google.com/d/optout.
