TASK: Remove references to `CommandResult` in return types

neos / neos-development-collection

The unified repository containing the Neos core packages, used for Neos development.

https://www.neos.io/

GNU General Public License v3.0

260 stars 220 forks source link

TASK: Remove references to `CommandResult` in return types #5061

Closed mhsdesign closed 1 month ago

mhsdesign commented 1 month ago

Followup to https://github.com/neos/neos-development-collection/pull/5059

Extracted from https://github.com/neos/neos-development-collection/pull/4988

Upgrade instructions

Review instructions

TODO rename *AndBlock methods

Checklist

[ ] Code follows the PSR-2 coding style
[ ] Tests have been created, run and adjusted as needed
[ ] The PR is created against the lowest maintained branch
[ ] Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
[ ] Reviewer - The first section explains the change briefly for change-logs
[ ] Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

bwaidelich commented 1 month ago

I'm not 100% sure whether it might be useful to have some return type that tells something about the published events (e.g. last sequence number) and maybe even about the updated/skipped projections

mhsdesign commented 1 month ago

yes but id rather discuss this in https://github.com/neos/neos-development-collection/pull/4988 or an issue? This was just meant so these files dont have to be adjusted with your pr. I checked out these files and voila ^^ For more transparency i would actually prefer at first to set some correlation metata in the events itself for node migration and then we can improve cli output. Your pr can very well introduce some Result object, but that doesnt have to be used yet here.

kitsunet commented 1 month ago

I am fine either way, CommandResult sounds right, we might just replace the inner workings and keep it around?

bwaidelich commented 1 month ago

I am fine either way, CommandResult sounds right

yes, my thinking. Does it make sense to remove the class if we re-add it a couple of PRs later?

mhsdesign commented 1 month ago

Okay so then lets not remove it via your pr https://github.com/neos/neos-development-collection/pull/4988: and we can close this for now and reopen for the case we want to get rid of it.