[regression] current master is umcompilable

Here's the place for discussion related to coding in FreeCAD, C++ or Python. Design, interfaces and structures.
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
User avatar
matthiaswm
Posts: 35
Joined: Wed Nov 18, 2020 11:41 am
Location: Düsseldorf, Germany

Re: [regression] current master is umcompilable

Post by matthiaswm »

Late to the game, but in my few days of writing some code for FC, I stumbled several times over CI issues in my Pull Requests, that were not caused by my code, but by rebasing and importing code from the master branch.

For example, my CI failed yesterday with CodeSpell

Code: Select all

Warning: src/Mod/AddonManager/addonmanager_devmode.py:87: commiters ==> committers
Error: Process completed with exit code 1.
This is of course no drama and can easily be fixed, but every time code is checked into master that fails the CI, all PR's that happen to rebase will fail as well and need attention. Time that could be used writing code.

We fixed this in Einstein (https://github.com/pguyot/Einstein) by never committing directly into `master`, but making everything into a Pull Request and wait for CI to finish before merging. Core developers can of course apply their own PR without a peer review, but by waiting for CI to finish first, silly bugs like the typo above are caught before they trigger the master CI or worse, a master that can't be built at all.

- Matthias

PS: happy to explain the detail. GitHub is a huge help here.
User avatar
Kunda1
Veteran
Posts: 13434
Joined: Thu Jan 05, 2017 9:03 pm

Re: [regression] current master is umcompilable

Post by Kunda1 »

Thanks for the post and observation. Worthy of further discussion IMHO.
Alone you go faster. Together we go farther
Please mark thread [Solved]
Want to contribute back to FC? Checkout:
'good first issues' | Open TODOs and FIXMEs | How to Help FreeCAD | How to report Bugs
User avatar
chennes
Veteran
Posts: 3884
Joined: Fri Dec 23, 2016 3:38 pm
Location: Norman, OK, USA
Contact:

Re: [regression] current master is umcompilable

Post by chennes »

Yeah, this one’s on me — I have gotten in the (probably bad) habit of just developing the Addon Manager directly on the mainline sine it can’t break compilation. With our newly aggressive CI settings, though, even Python can “break” things.
Chris Hennes
Pioneer Library System
GitHub profile, LinkedIn profile, chrishennes.com
User avatar
matthiaswm
Posts: 35
Joined: Wed Nov 18, 2020 11:41 am
Location: Düsseldorf, Germany

Re: [regression] current master is umcompilable

Post by matthiaswm »

chennes wrote: Sat Sep 24, 2022 3:35 pm Yeah, this one’s on me…
Oh, this was not meant to blame anyone. It was just one example, no big deal, easy to fix. With Strict CI in place, it was practical in my projects to not push directly to master. :-) some of my projects fail CI even if indent is wrong or brackets are in the wrong line. Very strict, but also very readable code… .
User avatar
uwestoehr
Veteran
Posts: 4961
Joined: Sun Jan 27, 2019 3:21 am
Location: Germany
Contact:

Re: [regression] current master is umcompilable

Post by uwestoehr »

chennes wrote: Sat Sep 24, 2022 3:35 pm Yeah, this one’s on me — I have gotten in the (probably bad) habit of just developing the Addon Manager directly on the mainline sine it can’t break compilation. With our newly aggressive CI settings, though, even Python can “break” things.
I see it relaxed. Nothing is actually broken. The compilation CI succeeds, so everyone can build master.
Personally I like the eager CI since typos are immediately seen and fixed.
User avatar
chennes
Veteran
Posts: 3884
Joined: Fri Dec 23, 2016 3:38 pm
Location: Norman, OK, USA
Contact:

Re: [regression] current master is umcompilable

Post by chennes »

Yes, I agree, I like the CI warning about these things: it works better when I wait for it to complete before going to bed, though!
Chris Hennes
Pioneer Library System
GitHub profile, LinkedIn profile, chrishennes.com
User avatar
matthiaswm
Posts: 35
Joined: Wed Nov 18, 2020 11:41 am
Location: Düsseldorf, Germany

Re: [regression] current master is umcompilable

Post by matthiaswm »

chennes wrote: Sat Sep 24, 2022 11:09 pm Yes, I agree, I like the CI warning about these things: it works better when I wait for it to complete before going to bed, though!
I fully agree. Our CI is even stricter, especially about formatting.

The issue is, that committing something as simple as a type typo to master causes all rebased PRs to fail CI, even if the PR itself is correct.

- Matthias

Edit: typo
Last edited by matthiaswm on Mon Sep 26, 2022 9:54 am, edited 2 times in total.
aapo
Posts: 615
Joined: Mon Oct 29, 2018 6:41 pm

Re: [regression] current master is umcompilable

Post by aapo »

matthiaswm wrote: Mon Sep 26, 2022 9:22 am The issue is, that committing something as simple as a type to master causes all rebased PRs to fail CI, even if the PR itself is correct.
That's a pretty funny typo considering the context! Was it intentional? :lol:
User avatar
matthiaswm
Posts: 35
Joined: Wed Nov 18, 2020 11:41 am
Location: Düsseldorf, Germany

Re: [regression] current master is umcompilable

Post by matthiaswm »

aapo wrote: Mon Sep 26, 2022 9:43 am
matthiaswm wrote: Mon Sep 26, 2022 9:22 am The issue is, that committing something as simple as a type to master causes all rebased PRs to fail CI, even if the PR itself is correct.
That's a pretty funny typo considering the context! Was it intentional? :lol:
I meticulously planned this typo long in advance to make me look sharp-witted and resourceful. Oh the irony. :oops:
Post Reply