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

TASK: WorkspaceWasPublished implement EmbedsWorkspaceName #5431

Open
wants to merge 1 commit into
base: 9.0
Choose a base branch
from

Conversation

mhsdesign
Copy link
Member

#5226 introduced a new EmbedsWorkspaceName interface for events but its not implemented by the WorkspaceWasPublished but all other publishing events. This pr fixes this inconsistency.

It was noted in #5406 (comment) that the introduction was deliberately not done because there is a certain ambiguity regarding returning "$sourceWorkspaceName" or "$targetWorkspaceName". Further we currently rely on WorkspaceWasPublished implementing this interface currently.

Still i wanted to elaborate why i think its correct and that its a good idea to have it:

I stumbled upon this while reading the code and this will be not fun to work with. The reason why i think the returning the sourceWorkspaceName is correct is because it is the intention of the command. Actually the fact that the targetWorkspaceName is part of the command is NEVER evaluated anywhere and might as well be removed (not really ^^) but its just metadata.

There is another related inconsistency regarding the RootWorkspaceWasCreated event: #5226 (comment) so it seems not always easy to mark the events correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant