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

Bring proto files inline with styling guide #3589

Merged
merged 9 commits into from
Nov 3, 2023
Merged

Bring proto files inline with styling guide #3589

merged 9 commits into from
Nov 3, 2023

Conversation

foot
Copy link
Collaborator

@foot foot commented Nov 3, 2023

Part of #3220

Add more rules to .protolint.yaml

  • RPCS_HAVE_COMMENT
  • FIELD_NAMES_LOWER_SNAKE_CASE

What changed?

  • The main motivation was to fix the camelCase -> snake_case style convention
  • To address the "require RPC method documentation" FIXME's have been added temporarily. We'll do a follow up PR to address descriptions on methods.

Why was this change made?

Tidy up before publishing API

How did you validate the change?

Follow ups

  • Add docs for each method that has a FIXME
  • Review existing docs

@foot foot added the exclude from release notes Use this label to exclude a PR from the release notes label Nov 3, 2023
@foot foot marked this pull request as ready for review November 3, 2023 12:15
Copy link
Contributor

@bigkevmcd bigkevmcd left a comment

Choose a reason for hiding this comment

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

Should we add an action to check these?

@foot
Copy link
Collaborator Author

foot commented Nov 3, 2023

Should we add an action to check these?

Turns out we do actually run it.

https://github.com/weaveworks/weave-gitops-enterprise/actions/runs/6745162221/job/18336814545?pr=3589#step:5:40

@bigkevmcd
Copy link
Contributor

Should we add an action to check these?

Turns out we do actually run it.

https://github.com/weaveworks/weave-gitops-enterprise/actions/runs/6745162221/job/18336814545?pr=3589#step:5:40

Ahh, awesome, thanks for tightening the rules!

@foot foot merged commit c0469c8 into main Nov 3, 2023
10 checks passed
@foot foot deleted the buf-lint branch November 3, 2023 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exclude from release notes Use this label to exclude a PR from the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants