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

Consolidate instruction casing lint rules #5046

Merged
merged 1 commit into from
Jun 17, 2024

Conversation

daghack
Copy link
Collaborator

@daghack daghack commented Jun 17, 2024

Currently, there are two lint checks that look at the casing of instructions, but they are largely redundant. This consolidates the two checks into a single casing check.

@daghack daghack self-assigned this Jun 17, 2024
@daghack daghack added this to the v0.14.1 milestone Jun 17, 2024
Copy link
Collaborator

@dvdksn dvdksn left a comment

Choose a reason for hiding this comment

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

@daghack
Copy link
Collaborator Author

daghack commented Jun 17, 2024

return fmt.Sprintf("Command '%s' should be consistently cased", command)
},
}
RuleFileConsistentCommandCasing = LinterRule[func(string, string) string]{
Copy link
Member

Choose a reason for hiding this comment

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

Maybe not needed yet, but generally I think we need to keep old rules/URLs in here so the docs are still generated for them. Otherwise old client will hit issue, click on a link and get 404

Copy link
Collaborator Author

@daghack daghack Jun 17, 2024

Choose a reason for hiding this comment

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

That's a good thought to bring up, and you're definitely right. There may be a case for adding some sort of Deprecated flag to the ruleset so that documentation can automatically note it as such as follow-up work.

@tonistiigi tonistiigi merged commit 09f1a48 into moby:master Jun 17, 2024
76 checks passed
@daghack daghack deleted the consolidate-case-checks branch June 20, 2024 15:23
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.

4 participants