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

refactor: move package_order to rezplugin system #1787

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

cfxegbert
Copy link
Contributor

I was trying to write a new package orderer and discovered that it is not part of the rezplugin system. This refactor moves the orderers defined in package_order.py to rezplugins.package_order plugins. Function have been added to package_order.py to remain backward compatible so if a user was directly importing an orderer it should still work.

Signed-off-by: Robert Minsk <robertminsk@yahoo.com>
@cfxegbert cfxegbert requested a review from a team as a code owner July 2, 2024 18:14
Copy link

codecov bot commented Jul 2, 2024

Codecov Report

Attention: Patch coverage is 83.33333% with 5 lines in your changes missing coverage. Please review.

Project coverage is 58.02%. Comparing base (e215a77) to head (09c12c9).
Report is 3 commits behind head on main.

Files Patch % Lines
src/rez/package_order.py 81.48% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1787      +/-   ##
==========================================
- Coverage   58.39%   58.02%   -0.38%     
==========================================
  Files         126      126              
  Lines       17205    17060     -145     
  Branches     3519     3490      -29     
==========================================
- Hits        10047     9899     -148     
- Misses       6491     6496       +5     
+ Partials      667      665       -2     

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

Signed-off-by: Robert Minsk <robertminsk@yahoo.com>
Signed-off-by: Robert Minsk <robertminsk@yahoo.com>
format: add blank line after license comment
@JeanChristopheMorinPerso
Copy link
Member

Hi @cfxegbert. Can you please explain the rationale behind this change? Context is important for us. It helps us document why things were changed, by who, the pros, cons, etc.

@cfxegbert
Copy link
Contributor Author

This was moved to the rez plugin system so a new package order could be written without having to modify the rez source code. Before a developer would have to patch the source code and apply the patch every release. By moving to the plugin system new package orders could be added without modifing the source.

@cfxegbert
Copy link
Contributor Author

I could also add the rez plugin hooks and not move the existing package orders if you think that would be better.

@chadrik
Copy link
Contributor

chadrik commented Jul 18, 2024

Scanline VFX is strongly in support of this feature.

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