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

Use newlines in config/generated.lst to prevent git conflicts #1245

Merged

Conversation

mbbush
Copy link
Collaborator

@mbbush mbbush commented Mar 28, 2024

Description of your changes

Currently, every pull request that adds a new resource kind modifies the one and only line of config/generated.lst, which means that git finds a conflict between any two such pull requests.

This change makes such PRs no longer be conflicting, by introducing newlines.

The only potential downside I can see here is that the conflict was serving as a sign that there were some additional changes that might impact the codegen, and you should rebase and rerun make generate. But they're a poor way to indicate that. If we really want to ensure that, we can enforce a linear history, but I think simply running CI on main is probably good enough.

This will also make it much easier to maintain that linear history, since you'll be able to git merge without having to rerun codegen, and the ci check-diff job will ensure you didn't miss anything.

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

@mbbush mbbush force-pushed the newlines-in-generated-lst branch from 2da4a5e to 64cb97e Compare April 22, 2024 03:45
@mbbush mbbush marked this pull request as ready for review April 23, 2024 17:07
@mbbush mbbush force-pushed the newlines-in-generated-lst branch from 64cb97e to 52e4450 Compare June 5, 2024 20:57
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.

Thank you for taking care of this @mbbush, LGTM.

@mbbush mbbush force-pushed the newlines-in-generated-lst branch from 52e4450 to 2975e3c Compare June 6, 2024 14:30
mbbush added 2 commits June 6, 2024 07:32
Signed-off-by: Matt Bush <mbbush@gmail.com>
Signed-off-by: Matt Bush <mbbush@gmail.com>
@mbbush mbbush force-pushed the newlines-in-generated-lst branch from 2975e3c to 11eca36 Compare June 6, 2024 14:33
@mbbush
Copy link
Collaborator Author

mbbush commented Jun 6, 2024

Thanks @turkenf. I rebased and resolved conflicts hopefully for the last time. I'll be AFK by the time the lint job finishes. Can you take care of clicking merge?

Signed-off-by: Matt Bush <mbbush@gmail.com>
@mbbush mbbush force-pushed the newlines-in-generated-lst branch from c55e4e3 to 9dc59d3 Compare June 6, 2024 14:45
@jeanduplessis jeanduplessis merged commit bda86c9 into crossplane-contrib:main Jun 6, 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.

3 participants