On 07/17/2017 04:20 AM, Amos Jeffries wrote: > On 17/07/17 06:14, Alex Rousskov wrote: >> Since we will switch to PR-only commit >> path (for master) as soon as Eduard's CI integration is ready, I suggest >> that we keep the current review-required setup to practice. We can >> always undo/relax it if we discover problems, and it is better to >> discover as much of them as possible _now_, in a simplified environment >> without CI. >> >> Do you think testing the PR review requirement before CI is ready is a >> good idea?
> I'm not sure what the problem you hint at might be, but testing to find > out is definitely a good idea whatever it is. I did not mean to hint at any known problem: When doing something new, like a new commit process, one is likely to run into some yet-unknown problems, and it would be nice to trouble shoot them in a simpler environment. The first such problem surfaced today: The "require Code Owners review" option blocked PRs because Squid has no Code Owners, as defined by Github[1]. I unchecked that option. [1] https://github.com/blog/2392-introducing-code-owners > I was envisioning the CI to be an extra pre-commit test done in parallel > with manual review so CI results are just treated in our process as an > automatic reviewer vote for/against commit - not as automatic acceptance > of all PR changes into master. Your summary matches my expectations. Master commits will be automated as well, but that is a separate step/problem, not directly related to CI. > Github: Merging can be performed automatically once the requested > changes are addressed. Please note that Github definition of "automation" appears to be different from mine in this context. AFAICT, Github considers manual commits done via its web interface "automation". I do not. BTW, while Eduard is working on proper Jenkins integration, I have enabled a basic "./bootstrap.sh && ./configure && make && make distcheck" test via Semaphore CI. You can see Github waiting for that test in your PR now... If you want to see the other side, I think you need to create a personal Semaphore CI account so that I can add you to the Squid Organization there. The CI check for your PR is probably stuck because the PR code has not been updated since CI testing was enabled. I tried to tickle Semaphore but failed, and added a comment about it to your PR. I know a similar check worked OK (eventually) for a _new_ PR that I did on a forked Squid repository before migrating the setup to the Squid project. Cheers, Alex. _______________________________________________ squid-dev mailing list [email protected] http://lists.squid-cache.org/listinfo/squid-dev
