AcademySoftwareFoundation / rez

An integrated package configuration, build and deployment system for software
https://rez.readthedocs.io
Apache License 2.0
941 stars 335 forks source link

refactor: move package_order to rezplugin system #1787

Open cfxegbert opened 3 months ago

cfxegbert commented 3 months ago

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.

codecov[bot] commented 3 months ago

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 :warning:
Additional details and impacted files ```diff @@ 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 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

JeanChristopheMorinPerso commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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

chadrik commented 2 months ago

Scanline VFX is strongly in support of this feature.