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 "Move RecursiveDataStructureTraverser to wp-cli/wp-cli package" #5866

Conversation

danielbachhuber
Copy link
Member

@danielbachhuber danielbachhuber commented Nov 10, 2023

Reverts #5864
See #5837

Let's have this live in its own repository

@danielbachhuber danielbachhuber added this to the 2.10.0 milestone Nov 10, 2023
@danielbachhuber danielbachhuber requested a review from a team as a code owner November 10, 2023 12:56
@danielbachhuber danielbachhuber merged commit a4303bc into main Nov 10, 2023
64 of 65 checks passed
@danielbachhuber danielbachhuber deleted the revert-5864-refactor/move-recursivedatastructuretraverser-to-wp-cli-package branch November 10, 2023 13:10
@schlessera
Copy link
Member

@danielbachhuber What's the reasoning behind creating a separate repository for this? This will add a whole heap of extra effort (package setup, release process, test setup & CI/CD, etc...). For these few files, why not just stick with adding it to the package that is the "framework" package? Did you see a specific use case for using this outside of WP-CLI?

@danielbachhuber
Copy link
Member Author

What's the reasoning behind creating a separate repository for this?

@schlessera So we can start using the classes right away, instead of waiting for the framework to be tagged, dealing with back compat, etc.

This will add a whole heap of extra effort (package setup, release process, test setup & CI/CD, etc...).

Our automation should solve this, no?

@schlessera
Copy link
Member

@schlessera So we can start using the classes right away, instead of waiting for the framework to be tagged, dealing with back compat, etc.

I usually deal with this by pulling in the main branch of the framework where needed, that's why they all come with a branch alias. During the release process, this is all corrected with stable tagged versions again.

Our automation should solve this, no?

Every package not only adds extra maintenance overhead and slows down CI/CD, but also introduces a new "API" in the form of an officially accessible package that has its own versioning/dependency tree hassles.

@schlessera
Copy link
Member

Moreover, I'd like to add that we cannot add a new package every time the framework would require an update that needs to immediately be accessible to a command repository.

@danielbachhuber
Copy link
Member Author

I usually deal with this by pulling in the main branch of the framework where needed, that's why they all come with a branch alias. During the release process, this is all corrected with stable tagged versions again.

@schlessera Ok, can you own setting this up? It sounds more complicated than what I proposed, but I don't have a strong opinion about this.

@schlessera
Copy link
Member

schlessera commented Nov 14, 2023

@danielbachhuber I set this up. It's not complicated at all, as all packages are already configured, it just means bumping the minimum framework version in the corresponding packages to a not-yet-released version (which then automatically pulls in dev-main). I added some further explanation here: wp-cli/entity-command#442 (comment)

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.

None yet

2 participants