Skip to content

Conversation

@sdn4z
Copy link
Collaborator

@sdn4z sdn4z commented Aug 19, 2025

Up until this PR, check_dependencies was returning a bool, making reference to if there were errors found or not. Now, it returns a list of the errors found instead.

Some other breaking addition where made at the same time, like changing the name of the argument dependencies_cli to dependencies.

BREAKING CHANGE: check_dependencies returns the list of dependencies and function arguments' names have changed

@sdn4z sdn4z requested a review from scastlara as a code owner August 19, 2025 13:44
@scastlara
Copy link
Collaborator

Why is this a breaking change? The executable twyn itself did not really change no? Maybe if you used twyn as a lib, but do we consider that part of the public interface?

@sdn4z
Copy link
Collaborator Author

sdn4z commented Aug 20, 2025

Why is this a breaking change? The executable twyn itself did not really change no? Maybe if you used twyn as a lib, but do we consider that part of the public interface?

I thought on making it breaking because of that actually! I guess we have to decide where to draw the line here. Imo, since we support different Python versions we implicitly acknowledge that it is indeed meant to be used as a library.

@sdn4z sdn4z force-pushed the check-deps branch 10 times, most recently from 14cf462 to 23d3c50 Compare August 20, 2025 14:20
@sdn4z sdn4z merged commit 2a11abe into elementsinteractive:main Aug 20, 2025
12 checks passed
sdn4z added a commit that referenced this pull request Aug 20, 2025
BREAKING CHANGE: check_dependencies now returns the list of dependencies
@sdn4z sdn4z changed the title BREAKING CHANGE: check_dependencies returns the list of dependencies feat!: check_dependencies returns the list of dependencies Aug 20, 2025
@sdn4z sdn4z added the feature label Aug 20, 2025
@sdn4z sdn4z deleted the check-deps branch September 10, 2025 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants