Skip to content

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
@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.

3 participants