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

Stop Auto sorting the composer.json #479

Closed
3 tasks done
devkinetic opened this issue Nov 19, 2019 · 4 comments · Fixed by #480
Closed
3 tasks done

Stop Auto sorting the composer.json #479

devkinetic opened this issue Nov 19, 2019 · 4 comments · Fixed by #480

Comments

@devkinetic
Copy link
Contributor

devkinetic commented Nov 19, 2019

Summary

Recently, the "sort-packages" value was added to the composer.json.

Motivation

This wreaks havoc when trying to use Bedrock as a clean upstream git repo, which is how we manage all of our wordpress installs. When merging from upstream, git is no longer able to reconcile what bedrock is adding/updating, vs what my template has, and what that individual website has. This is seamless, but only when packages are sorting in the json based on order of inclusion.

Example json ordering, using a top-to-bottom approach for clean git management.

  • Bedrock deps
  • Common deps we add to all projects
  • Individual deps per project

Additional context

Each time we merge from upstream now we need to explicitly ignore this line, and manually reconcile the changes to json, every time. Removing this parameter from the json allows the developer to enforce their own structure, rather than forcing ABC order that doesn't serve a functional purpose.

I feel like this addition was purely a cosmetic change, and works as a regression when trying to use within the context of CI/CD. My hope here is to keep Bedrock in a state where it can be easily integrated and updated in an automated manner.

@swalkinshaw
Copy link
Member

🤔 your logic makes sense but I also didn't see any sorting changes when we set that option? #456 is the PR and no packages were re-ordered.

The Composer docs implies it only affects sorting when using compose require as well? https://getcomposer.org/doc/06-config.md#sort-packages

@austinpray
Copy link
Contributor

Even if this is only causing problems for one person so far: I'm definitely on board with sticking closer to the composer defaults and removing this option. The more non-default behavior we have the more "surprising" it is to use Bedrock which should be considered a regression.

@devkinetic
Copy link
Contributor Author

So this made its way into our network, and when the application maintenance team pulled upstream, and then proceeded to update things the order was broken. Somehow this made its way through the merge request process, and now that we are at the next month. Nothing will merge cleanly due to reordering. I've since been ignoring the setting, and instructing my team to restore the previous ordering so we can keep the ball rolling. This ended up being a trivial, but time consuming endeavor. My hope is to ignore it.

If a developer wants to add this downstream, that can still work and merge cleanly. But having it flow from upstream every time complicates the ci/cd.

@swalkinshaw
Copy link
Member

👍 I'm good with removing it as well. Want to do a PR?

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 a pull request may close this issue.

3 participants