4 ms·
For ISO / PCI / SOX / HIPAA / contractual compliance in any "real" company there is a requirement of a review on all code shipped to production. "Real" because
by web007 10y ago
For ISO / PCI / SOX / HIPAA / contractual compliance in any "real" company there is a requirement of a review on all code shipped to production. "Real" because it's a huge pain in the ass, and you only do it if you have to work with another such company or if you're constrained by regulation to do so.
It doesn't matter the number of commiters, size of changes or frequency of release. You're bound to a process for reasons outside your control.
That's the strict version that I'd like them to implement, and what I thought we got out of enabling required reviews before reading in more detail. Even for one of their use-cases as described their flow won't work: allowing review of stylistic change to match project requirements. As it stands you can approve a change, then someone can screw with tabs-vs-spaces or reformat and merge on their own. Even rebasing to squash or reorganize doesn't require a subsequent approval. As suggested elsewhere, you'd have to automate the check (good idea anyway) and force it as a required test vs using approvals anyway.
Two things would improve the process. One would be implementing the strict mode, where merges can only happen after an approved review and any new commit (or even rebase) would trigger a reset. The second option would be to force two-man rule, where the committer can't merge their own PR. That fixes the compliance piece, and that's probably technically easier but logistically more complicated; person A can open a PR, then person B can approve AND make changes on the branch and still submit. That second flow is probably fine for OSS stuff, but is already covered by public repos and only allowing owners to merge. For a team it's kind of half-and-half, it depends on your requirements if it will pass compliance muster.