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

Alphabetize external name #1349

Merged
merged 6 commits into from
Jul 10, 2024

Conversation

mbbush
Copy link
Collaborator

@mbbush mbbush commented Jun 10, 2024

Description of your changes

config/externalname.go is one of the largest and most important hand-written files in this repository. Resources have been added (kind of) wherever in the file. Currently, when looking for a resource, or for several related resources, it is difficult to navigate or know when I've found all of them, and it looks very messy.

Additionally, if I'm adding a new resource, and put it at the bottom, this can create git conflicts with someone else adding an unrelated resource in the same place.

I'd love to have a linter that automates this, but I couldn't find one that could handle the way we group resources by their go sdk package name, and that keeps the resources together with their explanatory comments. So I just did it manually.

It is my hope that now that the file is alphabetized, we can enforce that users keep it alphabetized in code review.

In order to prevent clobbering the git history, I introduced a .git-blame-ignore-revs file with the SHA of my rename commit. Once approved, I will merge this with a merge commit to ensure that the sha doesn't change.

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable to ensure this PR is ready for review.

How has this code been tested

I programmatically compared the new and old versions of externalname.go and they only differ in comments, ordering of map keys, and whitespace.

There are new entries in externalnamenottested.go because I migrated a few resources that had been commented out in externalname.go to uncommented in externalnamenottested.go. But we don't currently use that list for anything.

Zero diff to the generated code confirms that this is a pure refactor with no meaningful changes.

mbbush added 3 commits June 9, 2024 17:11
Signed-off-by: Matt Bush <mbbush@gmail.com>
Signed-off-by: Matt Bush <mbbush@gmail.com>
Signed-off-by: Matt Bush <mbbush@gmail.com>
@mbbush mbbush marked this pull request as ready for review June 10, 2024 01:16
@mbbush mbbush mentioned this pull request Jun 10, 2024
3 tasks
Copy link
Collaborator

@turkenf turkenf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mbbush, LGTM.

@mbbush mbbush merged commit 787ec39 into crossplane-contrib:main Jul 10, 2024
11 checks passed
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 this pull request may close these issues.

2 participants