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

Project config should alphabetize entries. #4425

Closed
ryanpcmcquen opened this issue Jun 20, 2019 · 3 comments

Comments

@ryanpcmcquen
Copy link
Contributor

commented Jun 20, 2019

Description

Project config diffs can get unwieldy and ugly pretty quickly. In an enterprise environment where multiple developers are contributing to a Craft project, this can create some very ugly and misleading diffs of the project.yaml file. Alphabetizing entries would help mitigate this.

@brandonkelly

This comment has been minimized.

Copy link
Member

commented Jun 21, 2019

We used to sort everything in the project config, but that led to a couple issues such as #3997, so then we switched to only sorting top-level items.

As of the next release, we will also start sorting any arrays when all of the nested keys are UIDs, which seems safe.

@tprobinson

This comment has been minimized.

Copy link

commented Jun 21, 2019

To add to @ryanpcmcquen 's description of the unwieldy diffs, this can also cause merge errors-- if two sections that contain mostly the same stuff are moved around, Git's merger can tend to really munge them up since it's not data-structure-aware.

@brandonkelly With that issue you referenced, some of Craft's resources have keys like sortorder, right? Would it solve this to extend that approach to all (or more) things?

@brandonkelly

This comment has been minimized.

Copy link
Member

commented Jun 22, 2019

@tprobinson Yes, but the issue is that it was a breaking change for plugins as well. We’ll probably go back to sorting all indexed arrays by keys again in Craft 4. In the meantime I suspect that 1e808dc is going to go a long way in helping.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.