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

Only have normalized storage layouts in repository #1289

Merged
merged 2 commits into from
Sep 13, 2024

Conversation

area
Copy link
Member

@area area commented Sep 12, 2024

In #1258 I said

Strictly, we don't require both .storage-layouts directories here, but I opted to keep both. The first check in CI (which uses the plugin, and the original directory) will fail if storage slots are moved. The second check in CI (using my script and the normalized files) will fail if new variables have been added but not committed to the storage layout directories. It would also fail if storage slots had been moved, but I believe having the distinction between those two failure cases is worthwhile.

I am now of the opinion I was wrong, and longer believe that distinction is worthwhile at the cost of having huge diffs in PRs (such as in #1144, where the diff for a large feature is still dominated by non-meaningful changes in the .storage-layouts directory). I believe this will also encourage us to properly look at the (files representing the) storage changes in a PR, which I already found myself not doing.

I've also improved that second (now only) check, and it will catch the addition of new files that have not had storage slots added to the repository (as was the case in #1288 and was missed).

@kronosapiens kronosapiens merged commit c940c67 into develop Sep 13, 2024
2 checks passed
@kronosapiens kronosapiens deleted the maint/normalized-storage-layouts-only branch September 13, 2024 20:38
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.

2 participants