Skip to content

Conversation

devmotion
Copy link

@devmotion devmotion commented Aug 13, 2025

This PR extends #502 based on @DilumAluthge's suggestions in #502 (comment).

I added an option open_prs_for_extras that can be set to IfExistingCompatExtras() (the default), AllExtras() or NoExtras() [my gut feeling is that the names could be improved a bit]. With IfExistingCompatExtras(), only [extras] dependencies with existing compat entry are updated; with AllExtras(), all [extras] dependencies are updated (also those without existing compat entry); and with NoExtras(), [extras] dependencies are not updated.

I went with singleton types (basically a type-based enum) because 1) it felt more consistent with the existing EntryType and 2) it's not necessary to check whether the provided value is valid (which would be necessary if the API would be based on Symbols).

Fixes #166. Closes #502.

Copy link

codecov bot commented Aug 13, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (7ea4330) to head (98a2684).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #528   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           12        11    -1     
  Lines          412       417    +5     
=========================================
+ Hits           412       417    +5     

☔ 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.

@DilumAluthge
Copy link
Member

Could we make this experimental for now? That way, we can try it out, and if we don't like the API, we can change it?

@DilumAluthge
Copy link
Member

By "experimental", I mean explicitly document that we are allowed to break it in a minor (or even patch) release of CompatHelper.

@devmotion
Copy link
Author

I added a note.

@DilumAluthge DilumAluthge self-requested a review September 4, 2025 23:42
Co-authored-by: Dilum Aluthge <[email protected]>
@DilumAluthge DilumAluthge self-requested a review September 4, 2025 23:47
Ext = ["Bex_jll", "Skix"]

[extras]
Baz = "ea10d353-3f73-51f8-a26c-33c1cb351aa5"
Copy link
Member

Choose a reason for hiding this comment

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

Any particular reason for moving Baz from [deps] to [extras]?

Copy link
Author

Choose a reason for hiding this comment

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

I was just not creative enough and hence initially only moved around things. Only later I noticed that an additional dependency is needed to cover all cases.

@devmotion
Copy link
Author

@DilumAluthge this PR is ready for another round of review 🙂

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.

check "extras" not just "deps" ?
3 participants