Skip to content

Fix handle max size limitation#5

Merged
sbidoul merged 1 commit intoacsone:label-pr-modified-addonsfrom
grap:FIX-handle-max-size-limitation
Apr 2, 2026
Merged

Fix handle max size limitation#5
sbidoul merged 1 commit intoacsone:label-pr-modified-addonsfrom
grap:FIX-handle-max-size-limitation

Conversation

@legalsylvain
Copy link
Copy Markdown

No description provided.

@legalsylvain legalsylvain force-pushed the FIX-handle-max-size-limitation branch from a3aacf0 to 791f871 Compare April 1, 2026 23:37
@legalsylvain legalsylvain marked this pull request as draft April 1, 2026 23:37
@legalsylvain legalsylvain force-pushed the FIX-handle-max-size-limitation branch 2 times, most recently from b9a64d5 to 088560b Compare April 1, 2026 23:53
@legalsylvain legalsylvain marked this pull request as ready for review April 2, 2026 08:21
@legalsylvain
Copy link
Copy Markdown
Author

@sbidoul

Note : deployed in production, and works like a charm.
CI is broken because of change in cassettes.

If you think the original feature (OCA#300) is interesting to be merge, we can take a time to fix the test.


new_labels = set()
for modified_addon in modified_addons:
label_name = f"addons:{modified_addon}"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
label_name = f"addons:{modified_addon}"
label_name = f"addon:{modified_addon}"

Without s. Maybe we could even shortent it a bit more (ad: ? mod:) ?

Can you factor out the function that computes the shortened label?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(ad: ? mod:) ?

Of course !

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • added new config MODULE_LABEL_COLOR

@legalsylvain legalsylvain force-pushed the FIX-handle-max-size-limitation branch from 88887c7 to 2f29361 Compare April 2, 2026 10:07
@sbidoul
Copy link
Copy Markdown
Member

sbidoul commented Apr 2, 2026

I'll update the cassette. Can you add a couple of unit tests for _compute_label_name ?

@legalsylvain legalsylvain force-pushed the FIX-handle-max-size-limitation branch from 2f29361 to 4257aca Compare April 2, 2026 10:22
if label name is too long, it is cut, and ended by an hash of the module name.
the module name is present in the description of the label

- factor out function that computes the shortened label
- use 'mod:' instead of 'addon:'
- add MODULE_LABEL_COLOR as new configuration

Co-authored-by: Stéphane Bidoul <stephane.bidoul@acsone.eu>
@legalsylvain legalsylvain force-pushed the FIX-handle-max-size-limitation branch from 73c06aa to fca5c9d Compare April 2, 2026 10:30
@legalsylvain
Copy link
Copy Markdown
Author

legalsylvain commented Apr 2, 2026

Hi @sbidoul : done (except cassette change to do)
thanks a lot for your reactivity and your review.

General remark on this new feature : The current implementation tag PR that are changing any module files (and related files in setup).

I'd like to add an argument (exclude_autogenerated_files = False) in the function git_modified_addons, to return the list of module really changed.

I mean, as a maintainer of the module, I don't care to have the PR that are only changing autogenerated file in one of "my" module. (index.html, main README.rst, toml, etc...).

if a PR apply index template changes (like the OCA banner), it will tag all modules. that is not necessary (and wrong in my opinion), as there are "technical PR".

WDYT ? (not a blocking point, a nice to have IMO)

In fact, if this option is added, I'd like to enable it also for the "mention maintainer" feature.

@sbidoul sbidoul merged commit 640cc2d into acsone:label-pr-modified-addons Apr 2, 2026
1 check failed
@sbidoul
Copy link
Copy Markdown
Member

sbidoul commented Apr 2, 2026

It probably makes sense to exclude some PRs from various bot actions. Maybe open another issue about that?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants