Andrea and Peter, I think you raised an important point about the CI: depending on what we decide, they are going to be even more critical to our workflow.
I think the last few days have been outliers in terms of CI stability: the recent changes do touch on key parts of our build, so I think some level of instability it's OK until we sort out all the loose ends. We still have the eventual (OOM-related?) hang-up, but it seems to me that it's decreasing in frequency. In the end, I think if we adjust a bit the way we are working, it may even help with sorting out those instabilities: if we can bisect the code more easily, then it's going to be easier* to inspect those behaviors. *Obs. (OFF-TOPIC): In the future, we might even discuss with Infra about installing auto-bisect plugins on Jenkins or creating an auto-bisect job (I'll happily donate my scripts when we get to that point). Kind regards On Fri, Jun 23, 2023 at 5:19 PM Andrea Cosentino <anco...@gmail.com> wrote: > 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 > > > > > > > > -- Otavio R. Piske http://orpiske.net
