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

(Re)Moved Profile It command #1491

Merged
merged 3 commits into from Dec 14, 2023
Merged

Conversation

JanBliznicenko
Copy link
Contributor

@JanBliznicenko JanBliznicenko commented Dec 11, 2023

Removed ProfileIt command as it is being moved to NewTools and implemented there. Part of efforts towards resolving issue pharo-project/pharo#15795

@JanBliznicenko JanBliznicenko marked this pull request as ready for review December 13, 2023 12:49
@JanBliznicenko
Copy link
Contributor Author

JanBliznicenko commented Dec 13, 2023

I believe those tests failed because it needed the changes in the pharo repository that were not merged until now. Could you re-run the failed action, please?

@MarcusDenker
Copy link
Contributor

There are now failing:

 ✗ #testExternalBasicToolsDependencies (28ms)
 ✗ #testExternalIDEDependencies (69ms)
 ✗ #testExternalSpec2Dependencies (14ms)
 ✗ #testExternalUIDependencies (18ms)

@JanBliznicenko
Copy link
Contributor Author

JanBliznicenko commented Dec 13, 2023

There are now failing:

 ✗ #testExternalBasicToolsDependencies (28ms)
 ✗ #testExternalIDEDependencies (69ms)
 ✗ #testExternalSpec2Dependencies (14ms)
 ✗ #testExternalUIDependencies (18ms)

I see. What do these tests tell us? That there are dependencies that are not being loaded by baselines?

Spec2's class SpCodeProfileItCommand now depends on ProfilerUI's class StProfilerPresenter (in order to open the profile presenter).

Maybe we can move the SpCodeProfileItCommand to the ProfilerUI package? ProfilerUI depends on Spec2 anyway. ProfilerUI's baseline BaselineOfProfilerUI does not depend on anything, not even Spec2, but all the ProfilerUI presenters depend on Spec2.

@JanBliznicenko
Copy link
Contributor Author

JanBliznicenko commented Dec 14, 2023

If we move ProfilerUI to NewTools (pharo-project/pharo#15772), it would make even more sense to move the command to ProfilerUI as well. I will modify this pull request to only remove the command from here.

@JanBliznicenko JanBliznicenko changed the title implemented Profile It command (Re)Moved Profile It command Dec 14, 2023
@jecisc jecisc merged commit e17c2aa into pharo-spec:Pharo12 Dec 14, 2023
2 checks passed
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.

None yet

3 participants