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

Revert "Revert "Remove UIManager dependencies from Compression."" #16106

Closed

Conversation

guillep
Copy link
Member

@guillep guillep commented Feb 7, 2024

Rework of #16068 by @hernanmd .

The original PR broke the build. But this is going to get fixed :)

@guillep guillep marked this pull request as draft February 7, 2024 11:17
@hernanmd
Copy link
Member

hernanmd commented Feb 7, 2024

Can you comment what's the core change?
Because I tried to add in BaselineOfBasicTools:

		spec package: 'System-Spec' with: [ spec requires: #('NewTools-Core') ].

But that raised another error Error: Name not found: NewTools-Core while loading the Baseline

@hernanmd
Copy link
Member

hernanmd commented Feb 8, 2024

I re-take this PR here: pharo-spec/NewTools#683, they're essentially the same changes, but with reduced dependency to SpApplication, it is simpler to check and does not use the System-Spec approach.

So I think this PR can be closed if you agree.

@guillep
Copy link
Member Author

guillep commented Feb 8, 2024

Can you comment what's the core change?

I don't understand the question, could you rephrase?
Do you mean why reverting fixed the build?

Because I tried to add in BaselineOfBasicTools:

		spec package: 'System-Spec' with: [ spec requires: #('NewTools-Core') ].

But that raised another error Error: Name not found: NewTools-Core while loading the Baseline

Adding the requires clause does not fix the issue, it just adds a new one.
Each baseline only sees what's defined in itself, and in this case BaselineOfBasicTools does not define 'NewTools-Core'

@guillep guillep closed this Feb 8, 2024
@jecisc jecisc deleted the revert-16103-revert-16068-compression_ui-dep branch July 18, 2024 09:44
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.

2 participants