As of today, the CI build (both Jenkins and Github action) is less reliable than building locally.
So, we could discuss if we could add some better rules in terms of grouping commits or use mandatory PRs, but I don't think we are at that level of trust with CI. Il giorno ven 23 giu 2023 alle ore 17:13 Peter Palaga <ppal...@redhat.com> ha scritto: > Thanks a lot Otavio, for bringing this up! > > I can only second Nicolas that CI should not be circumvented unless > there is a serious reason. > Second, PRs should ideally contain one compilable commit per logical > change. As long as there is a just a single such commit in a PR, then > the CI can work as a good barrier against broken commits. > > Otherwise it is up to the PR author to make sure that the individual > commits are compilable too. I personally always try honor this rule. If > my PR contains multiple commits, I often take care to re-order and/or > fixup via git rebase --interactive. The readability of the history and > the time of my co-workers matter to me. > > Thanks, > > -- Peter > > On 22/06/2023 10:12, Nicolas Filotto wrote: > > Hi, > > > > For me, the best you can do to avoid this kind of problem is to avoid > committing directly into the target branch by always submitting a PR for > any change. And then, once the build is green, merge the PR with "Squash > and merge" which is the default action on Camel. The action "Rebase and > merge" should be avoided since the build result reflects the final state of > the branch, not the intermediate states. In case you need some changes to > fix the build, make sure to re-synchronize the source branch with the > target branch, especially in case the changes are not done on the same day > because the Camel branches are modified very frequently and each change can > potentially be in conflicts with yours. > > > > it is a quite cumbersome approach but I don't see any safer way. > > > > Regards, > > Nicolas > > ________________________________ > > From: Otavio Rodolfo Piske <angusyo...@gmail.com> > > Sent: Thursday, June 22, 2023 08:53 > > To: dev@camel.apache.org <dev@camel.apache.org> > > Subject: Re: Some thoughts on bisecting Camel and our git commit > practices > > > > Hi, > > > > Release-tagged commits don't help here. The whole point of bisecting is > > finding out precisely which commit introduced a certain behavior or bug, > so > > that I and other committers can diagnose, document and fix the problem. > > It's not possible to do that effectively with just release-tagged > commits. > > > > Also, I don't think each commit needs to be "release quality": that would > > be too much right now. I do think, however, that each commit should be > > compilable. Consider, for instance, the following commit pattern I notice > > when bisecting the code: > > > > commit1: TASK111: did some stuff ---> not compilable > > commit2: TASK111: did some stuff ---> compilable > > commit3: TASK111: did some stuff ---> not compilable > > commit4: TASK111: did some stuff ---> compilable > > > > Git bisect run is smart enough to skip commit2 and commit3 if given a > > proper exit code (that's what we do). However, the problem gets worse > once > > these compilable scripts start to pile up, such as: > > > > commit1: TASK111: did some stuff ---> not compilable > > commit2: TASK111: did some stuff ---> compilable > > commit3: TASK111: did some stuff ---> not compilable > > commit4: TASK111: did some stuff ---> compilable > > commit5: TASK222: did some stuff ---> not compilable > > commit6: TASK222: did some stuff ---> compilable > > commit7: TASK222: did some stuff ---> not compilable > > commit8: TASK222: did some stuff ---> compilable > > > > At some point, the bisect execution runs out of skippable commits and you > > have to: a) test manually, b) restart the bisect operation with different > > good/bad commits, or c) investigate the list of skipped commits one by > one. > > > > The same can happen when merging community contributions composed of > > multiple commits. > > > > So, I think we can improve here by doing 2 things: > > > > 1. When commiting the work on a task, making sure that every commit is > > compilable. For instance, in the pattern above, by squashing > > commit1+commit2, commit3+4, etc. > > 2. When merging community contributions with multiple PRs, squashing the > > contribution before merging - if suspicious that one or more of the > commits > > introduce uncompilable changes. > > > > If we start doing this now, bisecting will be much easier in the future. > > > > Kind regards > > > > On Thu, Jun 22, 2023 at 8:08 AM Petr Kuzel <petrku...@eurofins.com > .invalid> > > wrote: > > > >> Hi Otavio, > >> > >> the codebase does not have the release quality > >> after each commit. Test-driven development could > >> even invite devs to separately commit failing tests > >> as a proof they are effective. > >> > >> On contrary the release-tagged commits should > >> be safe. Would not help you to focus > >> on the release-tagged commits only, pls? > > > > > >> Hope it helps > >> Cc. > >> > >> -- > >> Mr. Petr Kužel, Software Engineer > >> Eurofins International Support Services s.à r.l. > >> Val Fleuri 23 > >> L-1526 LUXEMBOURG > >> > >> -----Original Message----- > >> From: Otavio Rodolfo Piske <angusyo...@gmail.com> > >> Sent: Wednesday, June 21, 2023 11:17 > >> To: dev <dev@camel.apache.org> > >> Subject: Some thoughts on bisecting Camel and our git commit practices > >> > >> > >> Folks, > >> > >> You may have seen that PRs constantly report failures when testing > changes > >> on core. This is almost always caused by flaky tests. > >> > >> I have been trying to find the root causes of those flaky tests. Quite > >> often this involves going all the way to early versions of Camel 3 to > find > >> stable executions of those tests. > >> > >> The investigation itself is mostly automated by using git bisect run ... > >> This significantly improves the developer ability to find the root > causes > >> of issues. > >> > >> Despite all this automation, however, the task has been ... quite > difficult > >> and lengthy. Even considering the extensive resources I have access to > >> (thanks to my employer), bisecting a single test failure can take > several > >> *days*. > >> > >> In particular, this investigation has been difficult because we have a > huge > >> amount of dirty commits on the tree: partially modified work, > uncompilable > >> changes, broken tests and more. > >> > >> While it's NOT my intention to suggest introducing/discussing a full > blown > >> clean commit policy, I do wonder if we have some room to discuss how we > can > >> improve the way we work to ensure that the tree is compilable and > somewhat > >> testable (and, possibly, enforce that). > >> > >> We cannot fix the git history of the project, but we can ensure that > future > >> investigation can be made much easier in the future. > >> > >> So, folks, does anyone have any suggestions about how we could improve > >> here? > >> > >> Kind regards > >> -- > >> Otavio R. Piske > >> > https://urldefense.com/v3/__http://orpiske.net/__;!!CiXD_PY!SZvpton5vuSh5elDATUqZmerIhuL7ZGmoTwQB-66e3u34QRJzR2DoTvCjF3miU2oF4glDqKC5VnT439E0B0$ > >> > > > > > > -- > > Otavio R. Piske > > > https://urldefense.com/v3/__http://orpiske.net__;!!CiXD_PY!SZvpton5vuSh5elDATUqZmerIhuL7ZGmoTwQB-66e3u34QRJzR2DoTvCjF3miU2oF4glDqKC5VnTh1-lNIM$ > > > > As a recipient of an email from the Talend Group, your personal data > will be processed by our systems. Please see our Privacy Notice < > https://www.talend.com/privacy-policy/> for more information about our > collection and use of your personal information, our security practices, > and your data protection rights, including any rights you may have to > object to automated-decision making or profiling we use to analyze support > or marketing related communications. To manage or discontinue promotional > communications, use the communication preferences portal< > https://info.talend.com/emailpreferencesen.html>. To exercise your data > protection rights, use the privacy request form< > https://talend.my.onetrust.com/webform/ef906c5a-de41-4ea0-ba73-96c079cdd15a/b191c71d-f3cb-4a42-9815-0c3ca021704cl>. > Contact us here <https://www.talend.com/contact/> or by mail to either of > our co-headquarters: Talend, Inc.: 400 South El Camino Real, Ste 1400, San > Mateo, CA 94402; Talend SAS: 5/7 rue Salomon De Rothschild, 92150 Suresnes, > France > > > >
