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

Add protobuf linter #314

Merged
merged 1 commit into from
Feb 15, 2019
Merged

Add protobuf linter #314

merged 1 commit into from
Feb 15, 2019

Conversation

husio
Copy link
Contributor

@husio husio commented Feb 12, 2019

Add make lint that will run protobuf linter on our codebase.

After finishing #262 all linter
errors should be gone and we can add make lint to our CI.

resolve #302

Add `make lint` that will run protobuf linter on our codebase.

After finishing #262 all linter
errors should be gone and we can add `make lint` to our CI.

resolve #302
@husio husio requested a review from alpe February 12, 2019 10:27

prototool-bin:
ifndef $(shell command -v prototool help > /dev/null)
@@curl -sSL https://github.com/uber/prototool/releases/download/v1.3.0/prototool-$(shell uname -s)-$(shell uname -m) -o $(HOME)/.bin/prototool
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a better place than $HOME/.bin to store it? I wonder how this should work on CI.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is a Go project, you could also store it via go install in the GOPATH

@codecov-io
Copy link

codecov-io commented Feb 12, 2019

Codecov Report

Merging #314 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #314   +/-   ##
======================================
  Coverage    71.5%   71.5%           
======================================
  Files         146     146           
  Lines        6335    6335           
======================================
  Hits         4530    4530           
  Misses       1365    1365           
  Partials      440     440

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 d2d9af1...a2a1d28. Read the comment docs.

Copy link
Contributor

@alpe alpe left a comment

Choose a reason for hiding this comment

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

Req: Can you execute it during travisCI build, please?


prototool-bin:
ifndef $(shell command -v prototool help > /dev/null)
@@curl -sSL https://github.com/uber/prototool/releases/download/v1.3.0/prototool-$(shell uname -s)-$(shell uname -m) -o $(HOME)/.bin/prototool
Copy link
Contributor

Choose a reason for hiding this comment

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

It is a Go project, you could also store it via go install in the GOPATH

@husio
Copy link
Contributor Author

husio commented Feb 15, 2019

I can add linter to makefile and CI when #312 is merged. Right now it's failing the build.

I think once I will add it to CI, I might have to alter the configuration as well (ie. how the binary is installed).

@alpe
Copy link
Contributor

alpe commented Feb 15, 2019

👍 good work. Very much appreciated!

@alpe alpe merged commit a3238d5 into master Feb 15, 2019
@alpe alpe deleted the protobuf_linter_issue_302 branch February 15, 2019 11:43
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.

Add prototool linter to build process
3 participants