-
-
Notifications
You must be signed in to change notification settings - Fork 119
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RFC] Red CI and how to go ahead #299
Comments
Skipping CI shouldn't be an option. I think at settings level, you can prevent to merge directly things like in odoo/odoo. |
btw thanks a lot @legalsylvain for doing this. It's good to see them green 🥂 |
I'm fully agree with that and I don't do the same thing at all with the OCA/pos repo I maintain. But in that case, better a partial CI that no CI. ;-)
I think blocking that possibility could block some maintenance operation. (for exemple, if oca quality maintainers decide to add some rule / change in default configuration, they want to push on all the head branches anyway. Don't you think ? Better to wait the point of view of @alexis-via who is the main commiter of this repository.
thanks ! Quite boring job, but I'm glad I did this. l10n-france is the only repo that is always red and that I use in production. It's not a comfortable situation. |
Happy to see some interest in having Travis/pre-commit green on OCA/l10n-france. When I opened a ticket about red tests in 2018 on v10.0 (#155), nobody cared... happy to see that it's not the case any more!
The other issues are small details that are very easy to solve. I must say that PR #298 is completely ridiculous. It doesn't try to solve the errors, it just disables most of the modules. @legalsylvain You say you have experience with Travis/pre-commit setup (I don't have this experience myself), so it would have been very easy for you to solve most of the errors. OK to exclude l10n_fr_fec_oca because this one has a lot of warnings which are difficult to solve, but the others are pretty easy to fix for someone with experience on travis/pre-commit. I was expecting a much better contribution from you on this topic, I'm very disappointed. I just hope that it was a temporary PR waiting for a second PR that would solve the problems themselves (at least the easy ones). |
That one is easy and I told you about it a few days ago, it's similar to https://github.com/OCA/l10n-france/blob/14.0/.pre-commit-config.yaml#L10 except you would specify the exact file or sub-directory wou want to exclude from the rewrite (which is legit). Also, I tend to think partial CI is better than no CI, so it had to start at some point and I prefer to see the half full glass. As for merging in 24h or setting your modules as Alpha yes I agree this was borderline. Good luck with the other fixes. |
Folks please keep cool. I know CI is sometimes annoying... but in repos shared between multiple maintainers as we have in OCA, keeping it green is a matter of courtesy for other maintainers. And then ocabot merge command will run all the checks for you. So please forget the merge button. And if you are in a hurry, leave the PR open and install at your customer with gitaggregate or A few general notes from my side:
Regarding the specific issues in this repo, I did not look at the test failures if there are any. I did run pre-commit to see where things stand. It's not so bad. Here are the issues and suggested (easy) solutions:
|
There hasn't been any activity on this issue in the past 6 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
I'm sorry to reopen this issue. The CI was all green some monthes ago. But for now :
So I see again PRs merged directly without using ocabot command. How to go ahead ? CC : @OCA/local-france-maintainers |
You cannot say "CI was all green some monthes ago" ; it is not true. I made the work to have Travis green on v14 (PR #302) and on v12 (PR #303). AFAIK, nobody made the work to make Travis green on other branches. For example, as explained in one of my comment, v10 is red because of a failed check in guewen's module hr_holidays_jour_ouvrable (cf #155) and nobody solved it so far. v14 and v15 are green. On v12, travis is green (only runbot is red). I believe those branches are the most important... |
you're right ! I don't remember that point, but the work regarding greenification was only about the main branches.
yes, for me that is an important branch, and it was green. What your proposal ? conserve green branch on 14 & 15 and let other branches red ? thanks. |
I am willing to do the work for v10.0. Who is willing to do it for v11 and v13 ? |
I dont use 11 and 13 and my oca investment is more about openupgrade and ocabot. But i can try to fix v12 branch I still use. |
arf, ok. If nobody is motivated, then we'll keep these old branches red and just focus on keeping v14+ branches green. |
Hi all,
Travis Status
Runbot Status
That is quite problematic, mainly because all the contribution PRs are so red, even if there are valid. Included trivial ones. (see)
The work to make all the branch green is big, and trying to fix a problems raise other problems. see : #286 by @ivantodorovich.
flake8 / pylint / pre-commit issues ; no references to oca repositories ; trouble in python lib ; other errors ; ...
To go ahead, I so quick-fixed
.travis.yml
file. (EXCLUDE syntax).pre-commit-config.yaml
file.development_status
key toAlpha
So CI is now green, and it should stay green !
However, it is not respecting OCA guidelines. (even for alpha modules, see oca conventions)
I don't know how to continue in the good way.
In the meantime :
ocabot rebase
for your PR against 12.0 and 14.0 branches.CC : recent contributors : @alexis-via, @kevinkhao, @bealdav, @ivantodorovich
CC : OCA quality-tools maintainers : @sbidoul, @pedrobaeza
The text was updated successfully, but these errors were encountered: