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

Update users of IceTipStandardAction to use #newXAction + #executeWithContext: pattern (fixes #1779) #1781

Merged
merged 3 commits into from
Jan 19, 2024

Conversation

daniels220
Copy link
Contributor

@daniels220 daniels220 commented Dec 16, 2023

Fixes nil DNU #application errors for package reload/remove/unload, tag delete, and method version load (fixes #1779).

  • Also remove extraneous IceRepositoryModified when the action already has #onSuccessRepositoryModified.
  • Add missing ^return in IceTipVersionHistoryModel>>retrieveHistory.
  • Remove IceTipStandardAction>>execute: as execution without a context will ~always fail and the pattern is to assign the action first, then the context at time of execution. No references remain in Iceberg packages, and in fact all remaining references are self-sends, so clearly aren't sent to IceTipStandardAction.
  • Consistent classification for #execute methods in IceTipCommand hierarchy

…hContext: pattern (fixes pharo-vcs#1779)

Fixes nil DNU #application errors for package reload/remove/unload, tag delete, and method version load.
Also remove extraneous IceRepositoryModified when the action already has #onSuccessRepositoryModified.
Add missing ^return in IceTipVersionHistoryModel>>retrieveHistory

Remove IceTipStandardAction>>execute: as execution without a context will ~always fail and the pattern is to assign the action first, then the context at time of execution. No references remain in Iceberg packages, and in fact all remaining references are self-sends, so clearly aren't sent to IceTipStandardAction.
@Ducasse
Copy link
Collaborator

Ducasse commented Dec 16, 2023

Thanks for the PR.

@guillep
Copy link
Member

guillep commented Jan 18, 2024

Thanks!

@guillep
Copy link
Member

guillep commented Jan 18, 2024

Oh there are conflicts, I'll push them to the end of the queue

@guillep
Copy link
Member

guillep commented Jan 18, 2024

Hi @daniels220 I resolved the conflict locally but it seems you did not give rights to push to your branch so I cannot fix it :)

@daniels220
Copy link
Contributor Author

Ah, I didn't realize you were going to resolve them, so I went ahead and did so. No actual conflicts—doing a back-merge in Iceberg didn't flag anything—just Git not being very syntax-aware.

@guillep
Copy link
Member

guillep commented Jan 19, 2024

Ah, I didn't realize you were going to resolve them, so I went ahead and did so. No actual conflicts—doing a back-merge in Iceberg didn't flag anything—just Git not being very syntax-aware.

Sure no problem!

Thanks again!

@guillep guillep merged commit 7880973 into pharo-vcs:dev-2.0 Jan 19, 2024
0 of 3 checks passed
@daniels220 daniels220 deleted the ensure-context-for-actions branch January 29, 2024 23:04
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.

Reload raised an error because context is nil
3 participants