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

[RFC] new tools to mark PR as blocked or OK automatically #3979

Open
legalsylvain opened this issue Jul 2, 2023 · 9 comments
Open

[RFC] new tools to mark PR as blocked or OK automatically #3979

legalsylvain opened this issue Jul 2, 2023 · 9 comments

Comments

@legalsylvain
Copy link
Contributor

legalsylvain commented Jul 2, 2023

Hi @OCA/openupgrade-maintainers

Since I've been working on Openupgrade, I'm regularly annoyed to see PRs reviewed (or very simple), but not merged.
the problem is that to merge PRs in OpenUpgrade, you have to follow the dependency ordering. (to merge "product", you need to have merged "uom").

so I spent a few hours making a script that automatically scans all OpenUpgrade open PRs, marking them as "Blocked by dependency" or "Dependency OK". (new green label).

As a result,

image

So once portal is merged, the following modules can be marked as Dependency OK (if all the other dependencies are OK):
mail_group, website, test_mail_full, digest, mass_mailing_sms, auth_password_policy_portal, account, event, project, website_crm_partner_assign, payment, website_payment, portal_rating, auth_totp_portal
etc ...

note:

For the moment, the tools is available here : https://github.com/legalsylvain/openupgrade-extra-tools as a simple script. I'll execute it regularly. You can do it also, (as soon as you have maintainer accreditation).

CC : @vdewulf, @remytms

@huguesdk
Copy link
Member

huguesdk commented Jul 3, 2023

great! simple and effective. thanks @legalsylvain!

@remytms
Copy link
Contributor

remytms commented Jul 3, 2023

great tool ! 🚀

@legalsylvain
Copy link
Contributor Author

Hi @remytms and @huguesdk. Thanks for your feedbacks !

FYI, I just updated the script to add another feature :

  • For each opened PRs that are "blocked", enumerate the list of the PRs that we have to review first.
    For exemple : in the sale_stock PR ([16.0][OU-ADD] sale_stock #4015 (comment)),
    add a link to sale and stock_account PR.

image

Note : the text will be updated, each time I run the script and if the related PRs has been merged.

@hbrunn
Copy link
Member

hbrunn commented Jul 11, 2023

this is great! Can we shoehorn this into a github action to run on every merge/PR opening?

@legalsylvain
Copy link
Contributor Author

legalsylvain commented Jul 11, 2023

  • can be on every merge, indeed.
  • regarding every opening, it doesn't work, because my script rely on the main "Migration to X" issue. So we need to wait a maintainer has written "ocabot migration xxx". run on each comment looks a little bit heavy.

Alternatively, we can just run it each day. What do you think ?

Side question, for the time being, it is a custom repo, and that is my login that is changing / adding comment. do you want this repo to move under oca umbrella ?

@hbrunn
Copy link
Member

hbrunn commented Jul 11, 2023

triggering the action on merge + once per day sounds good to me. and if it's an action in this repo, no problems with authentication, right?

@legalsylvain
Copy link
Contributor Author

no problems with authentication, right?

I have to pass some authentication token to the script. don't know if Openupgrade contains oca-github-bot token...

https://github.com/legalsylvain/openupgrade-extra-tools/blob/master/check_dependency.py#L9

@hbrunn
Copy link
Member

hbrunn commented Jul 11, 2023

that's just a configuration in the repo settings, where we can choose if the workflow's token has write permissions. alas, I don't have access to that for openupgrade

@FernandoRomera
Copy link

@legalsylvain

Thank you very much for your utility.

Do you think it would be possible to transfer this work to the "Migration to 17.0" page, let me explain:

For example, I would like to migrate l10n-spain/l10n_en_account_asset. When you run this utility, the migration issue may be updated. So that it would look something like this:

l10n-spain

  • l10n_en_account_asset: PR
    Depends on:
    • account_asset_management: PR

account-financial-tools

  • account_asset_management: PR
    Depends on:
    • report_xlsx_helper: PR
    • account (Don't include is Odoo Community)

reporting-engine

  • report_xlsx_helper: PR
    Depends on:
    • report_xlsx: PR

Then i will started by report_xlsx_helper, the last in the chain. Or if I look at any Migration to 17 page, I will know if all the dependencies are migrated or not.

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

No branches or pull requests

5 participants