Skip to content
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

Additional / extracted status checks / passes #109

Closed
xmo-odoo opened this issue Mar 20, 2019 · 6 comments
Closed

Additional / extracted status checks / passes #109

xmo-odoo opened this issue Mar 20, 2019 · 6 comments
Assignees
Labels

Comments

@xmo-odoo
Copy link
Collaborator

Currently checking CLA is done sequentially with other runbot tasks despite there being no real dependency on the runbot testing infrastructure & al, aside from testing the code. It might be useful to set up a separate status pipeline which would run on a different machine (runbot10 should have unused capacity, mergebot is pretty much idle), and would generate additional statuses (possibly even gh checks?) onto which mergebot could depend:

  • CLA, which could be extended to check all commits rather than just the last
  • linting commit messages
  • code linting and formatting (JS and / or python, possibly moving the pylint pass out of the runbot's sequence)
  • documentation CI
  • ease of adding further tasks?

Furthermore, it would allow only performing these checks for PRs as we usually don't care much about them in WIP branches (in which some of them don't even make sense e.g. all contributors pushing to dev repos should pass CLA by definition).

This could be either bespoke or done through e.g. a gitlab ci/cd instance or something like that, these needs don't seem super complex and ought be pretty much solved by existing solutions.

@odony
Copy link
Contributor

odony commented Mar 20, 2019

Sounds like a good idea 👍
One "further task" we desperately need is a lint check that looks for security-sensitive patterns in PRs (e.g. t-raw, cr.execute, sudo, *eval, _.sprintf, etc.), and adds/removes a tag on the PR, e.g. "Need Security Review". Mergebot would refuse to merge PRs with this tag unless someone with a "Security Review" role has approved it, either via r+ or maybe a specific/limited s+ 🤔

@odony
Copy link
Contributor

odony commented Mar 20, 2019

An external gitlab ci/cd might seem overkill though, the bulk of the work is to write the checks, we kind of have the pipeline boilerplate already I guess.

@xmo-odoo
Copy link
Collaborator Author

An external gitlab ci/cd might seem overkill though, the bulk of the work is to write the checks, we kind of have the pipeline boilerplate already I guess.

Might be. My understanding of e.g. gitlab pipelines is you can easily define "concurrent" check jobs, easily set up new workers connected to the base instance, that sort of things.

@Xavier-Do
Copy link
Contributor

Xavier-Do commented Jul 29, 2019

The asynchronous tests system is in place. I t think that most of the points here can be easily set up.

  • Cla check is actually very fast so no real need to do it async, even if right now it is the case in master and staging master.
  • linting commit messages: I have a little script that does that, it could be interesting to rewrite it and test it.
  • code linting and formatting (JS and / or python, possibly moving the pylint pass out of the runbot's sequence) right now, all tests are spitted in staging, with no particular choice. The difference is that pylint could be executed without any install I guess. But adding more lint and js linting could be very useful.
  • documentation CI it has been asked bu devs we can work on it
  • ease of adding further tasks it is already the case

@Xavier-Do
Copy link
Contributor

Except for the commit message validation, all done.

@KangOl
Copy link
Contributor

KangOl commented Oct 14, 2021

For this, upgradeci run gitlint on all commits of the PR (upgradeci only works at PR level, not on branches).
On the upgrade repo, it uses the committed config but the runbot may use its own config file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants