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

ci: add proto file comments linter #733

Merged
merged 11 commits into from
Jul 24, 2022

Conversation

MichaelDeSteven
Copy link
Contributor

What this PR does:

Which issue(s) this PR fixes:

see #623

Special notes for your reviewer:

Does this PR introduce a user-facing change?:


@codecov
Copy link

codecov bot commented Jul 23, 2022

Codecov Report

Merging #733 (29778eb) into main (c9e000d) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #733   +/-   ##
=======================================
  Coverage   60.12%   60.12%           
=======================================
  Files         128      128           
  Lines        6774     6774           
=======================================
  Hits         4073     4073           
  Misses       2310     2310           
  Partials      391      391           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c9e000d...29778eb. Read the comment docs.

@MichaelDeSteven MichaelDeSteven changed the title ci: add proto file comments ci: add proto file comments linter Jul 23, 2022
@MichaelDeSteven
Copy link
Contributor Author

MichaelDeSteven commented Jul 23, 2022

@seeflood the ci file and makefile is work it. The final step is adding comments to pass Proto Validation.
the runtime.proto has some unfamiliar message, maybe need your help to add it.

make/proto.mk Outdated Show resolved Hide resolved
@seeflood seeflood requested a review from Xunzhuo July 24, 2022 00:44
@MichaelDeSteven MichaelDeSteven requested a review from seeflood July 24, 2022 05:48
@MichaelDeSteven
Copy link
Contributor Author

MichaelDeSteven commented Jul 24, 2022

Hi @seeflood, @Xunzhuo
I use a easy way to install buf linter. Plz have a re-review.

buf.yaml Outdated Show resolved Hide resolved
make/proto.mk Outdated Show resolved Hide resolved
@Xunzhuo
Copy link
Member

Xunzhuo commented Jul 24, 2022

image

@MichaelDeSteven Resolve the lint errors, other than that, LGTM

Copy link
Member

@seeflood seeflood left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you all !

@seeflood seeflood merged commit e19e539 into mosn:main Jul 24, 2022
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