Skip to content

18.0 yac yet another changes#5

Merged
etobella merged 21 commits intodixmit:18.0-add-vcpfrom
grap:18.0-YAC-yet-another-changes
Mar 2, 2026
Merged

18.0 yac yet another changes#5
etobella merged 21 commits intodixmit:18.0-add-vcpfrom
grap:18.0-YAC-yet-another-changes

Conversation

@legalsylvain
Copy link

@legalsylvain legalsylvain commented Mar 1, 2026

some screenshots of the changes

image image image image

@codecov
Copy link

codecov bot commented Mar 1, 2026

Codecov Report

❌ Patch coverage is 78.15126% with 26 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (18.0-add-vcp@2faf3b4). Learn more about missing BASE report.

Files with missing lines Patch % Lines
vcp_odoo/models/vcp_rule.py 18.18% 9 Missing ⚠️
vcp_odoo/models/vcp_odoo_python_library.py 64.70% 6 Missing ⚠️
vcp_management/models/vcp_request.py 84.61% 2 Missing and 2 partials ⚠️
vcp_management/models/vcp_request_label.py 86.36% 2 Missing and 1 partial ⚠️
vcp_github/models/vcp_repository.py 60.00% 1 Missing and 1 partial ⚠️
vcp_odoo/models/vcp_odoo_bin_package.py 77.77% 2 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff               @@
##             18.0-add-vcp       #5   +/-   ##
===============================================
  Coverage                ?   84.30%           
===============================================
  Files                   ?       48           
  Lines                   ?     1198           
  Branches                ?      110           
===============================================
  Hits                    ?     1010           
  Misses                  ?      149           
  Partials                ?       39           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@legalsylvain legalsylvain force-pushed the 18.0-YAC-yet-another-changes branch 2 times, most recently from 628a7d4 to 62929c1 Compare March 2, 2026 00:47
@legalsylvain legalsylvain marked this pull request as ready for review March 2, 2026 01:21
@legalsylvain legalsylvain force-pushed the 18.0-YAC-yet-another-changes branch from 09cb190 to 7d44743 Compare March 2, 2026 01:23
@legalsylvain legalsylvain force-pushed the 18.0-YAC-yet-another-changes branch from 7d44743 to 69af62c Compare March 2, 2026 01:24
@legalsylvain
Copy link
Author

@etobella, @sebastienbeau : this one is ready for review. I integrated some improvment regarding badges, description, etc.

…to scheduled_information_update.

Add an helper.
Check the field by default, as it is the expected behaviour user want
…to scheduled_information_update for vcp.repository model.

    Add helpers. explicit limit in cron python code.
…heduled_branch_update for vcp.repository model.

    Add helpers. explicit limit in cron python code.
…if the target branch doesn't match the pattern
@legalsylvain legalsylvain force-pushed the 18.0-YAC-yet-another-changes branch from e3a57a2 to d6aa041 Compare March 2, 2026 13:14
<field name="model_id" ref="model_vcp_repository" />
<field name="state">code</field>
<field name="code">model._cron_update_branches()</field>
<field name="code">model._cron_update_branches(limit=1)</field>

Choose a reason for hiding this comment

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

If we set a limit by default, we may change the default interval type (like every 10 minutes of 30 minutes or 1 hours?) what do you think ? (same for other cron)

Copy link
Author

Choose a reason for hiding this comment

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

in that commit, I just "moved" the default. It was "invisible" and in the code, it is now visible and can be changed by UI, changing the "code" of the cron.

See : befb1ed#diff-b554761c27adb312a1ee5d6b213691f187a281132e08f4525ea9b16d7eb7ba1bL43-R43

About the value, I have absolutely no clue, and I think it depends on the context. I used the code to fetch all grap modules. (250, in custom repo). I had no problem, with the value !

Copy link
Member

Choose a reason for hiding this comment

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

If we want to use queue, we should set it in a glue module, it is not a required dependancy.

Copy link

@sebastienbeau sebastienbeau left a comment

Choose a reason for hiding this comment

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

Thanks for the UI improvement (adding a computed status is a must!)

An alternative of the limit on the cron may have to use queue_job. So the cron will be just here to create all the job and then the job will be processed one by one (or more depending on the config).

What do you think ? (I can do it)

@legalsylvain
Copy link
Author

Thanks for the UI improvement (adding a computed status is a must!)

An alternative of the limit on the cron may have to use queue_job. So the cron will be just here to create all the job and then the job will be processed one by one (or more depending on the config).

What do you think ? (I can do it)

As said, I didn't introduced limit concept. just move a default value from a hardcoded place to a changeable place.

Some vcp_queue_job could do the work. Feel free to implement it, if you think it's required. (as said for my "not so small" repos of 250 modules, no problem, I fetched all the history in some hours. Indeed, maybe for the OCA, we'll need some optimization.

@sebastienbeau
Copy link

Ok thanks for the feedback, let's merge it @etobella

@etobella etobella merged commit 373b7a3 into dixmit:18.0-add-vcp Mar 2, 2026
6 checks passed
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.

3 participants