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

Moved to GH actions; Added lint; Added issue/PR templates. #296

Merged
merged 5 commits into from Jun 18, 2020
Merged

Conversation

bwplotka
Copy link
Collaborator

Also pinned Go tools thanks to https://github.com/bwplotka/bingo

Signed-off-by: Bartlomiej Plotka bwplotka@gmail.com

@bwplotka bwplotka force-pushed the ci-actions branch 2 times, most recently from 9d3ee2c to 5b83c99 Compare May 30, 2020 14:36
@bwplotka
Copy link
Collaborator Author

Let's make sure the CI GH actions runs here

.github/stale.yml Outdated Show resolved Hide resolved
.github/stale.yml Outdated Show resolved Hide resolved
.github/stale.yml Outdated Show resolved Hide resolved
.github/stale.yml Outdated Show resolved Hide resolved
.github/stale.yml Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@bwplotka
Copy link
Collaborator Author

@bwplotka
Copy link
Collaborator Author

bwplotka commented Jun 1, 2020

Super thank your for awesome quick review, will address this afternoon! (:

@bwplotka bwplotka force-pushed the ci-actions branch 3 times, most recently from 7eb5620 to a6c363a Compare June 4, 2020 17:41
@bwplotka bwplotka mentioned this pull request Jun 4, 2020
18 tasks
@bwplotka bwplotka force-pushed the ci-actions branch 2 times, most recently from 533f0ef to 945fc83 Compare June 4, 2020 17:59
@irridia
Copy link
Contributor

irridia commented Jun 4, 2020

Be sure to update the import paths in the examples_test files, both for users and it confuses go mod.

@bwplotka
Copy link
Collaborator Author

bwplotka commented Jun 5, 2020 via email

@bwplotka bwplotka force-pushed the ci-actions branch 2 times, most recently from 635d8ed to 97f4f5a Compare June 11, 2020 13:44
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
@bwplotka
Copy link
Collaborator Author

Lot's of vert / lint issue to go through...

@bwplotka
Copy link
Collaborator Author

Should be fine now, PTAL @yashrsharma44 @johanbrandhorst

@codecov-commenter
Copy link

codecov-commenter commented Jun 11, 2020

Codecov Report

Merging #296 into v2 will decrease coverage by 4.65%.
The diff coverage is 90.90%.

Impacted file tree graph

@@            Coverage Diff             @@
##               v2     #296      +/-   ##
==========================================
- Coverage   84.01%   79.35%   -4.66%     
==========================================
  Files          30       30              
  Lines         932      930       -2     
==========================================
- Hits          783      738      -45     
- Misses        110      150      +40     
- Partials       39       42       +3     
Impacted Files Coverage Δ
chain.go 0.00% <ø> (-90.91%) ⬇️
interceptors/tags/fieldextractor.go 84.84% <ø> (-0.45%) ⬇️
interceptors/logging/payload.go 83.07% <50.00%> (+0.46%) ⬆️
interceptors/retry/retry.go 77.71% <100.00%> (-0.74%) ⬇️
wrappers.go 66.66% <0.00%> (-33.34%) ⬇️

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 6e2c2ac...c1c1b73. Read the comment docs.

@bwplotka bwplotka force-pushed the ci-actions branch 2 times, most recently from dd7379b to c1c1b73 Compare June 11, 2020 18:39
@bwplotka
Copy link
Collaborator Author

Close to be finished.. two last standing thigs:

  1. make lint works locally but not on GH Actions:
make[1]: Leaving directory '/home/runner/work/go-grpc-middleware/go-grpc-middleware'
cannot detected proto changes or other not formatted files
/bin/bash: run: command not found
M	grpctesting/gogotestpb/fields.pb.go
M	grpctesting/testpb/test.pb.go
Please commit or stash them.

This is super weird cannot detected proto changes or other not formatted files - looks like goimports does not format anything at the end 🤔

  1. Flaky time based tests: FAIL: TestRetrySuite/TestServerStream_PerCallDeadline_Succeeds (0.54s)

Some work to either mock time or increase timeouts ... Will look tomorrow if you want to help feel free to propose PR to this PR @https://github.com/yashrsharma44 I have time off tomorrow kind of. (:

@irridia
Copy link
Contributor

irridia commented Jun 11, 2020

go mod behaves and I can import and use the logging. \o/

I feel your pain on the tests...

Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
yashrsharma44 and others added 3 commits June 18, 2020 16:05
* Fixed linter detected issues.

Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>

* Add format-only flag

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>

* Modify the require_clean_work_tree function. Taken from - https://github.com/git/git/blob/master/git-sh-setup.sh#L211

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>

Added a git script

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>

chmod change

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>

changes to git-tree.sh

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>

Modified makefile

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>

clean up changes

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>

* Bump timeout deadline to 100ms

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>

Co-authored-by: Bartlomiej Plotka <bwplotka@gmail.com>
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
@bwplotka
Copy link
Collaborator Author

Should be good to go - will work on retry flakinesses in separate PR. @yashrsharma44 @johanbrandhorst (:

@yashrsharma44
Copy link
Collaborator

Thanks @bwplotka on working on this PR! I will try to make up for the lost time 🤓

@bwplotka bwplotka merged commit 880c174 into v2 Jun 18, 2020
@bwplotka bwplotka deleted the ci-actions branch June 18, 2020 17:31
@irridia
Copy link
Contributor

irridia commented Jun 18, 2020

Can you cut a new tag on branch v2, perhaps v2.0.0-rc.2? It'll make mod updates automatic-er.

@johanbrandhorst
Copy link
Collaborator

@yashrsharma44
Copy link
Collaborator

Hey @irridia! Can you share how did you correctly install v2 branch?

@irridia
Copy link
Contributor

irridia commented Jun 22, 2020

I've created a new release: https://github.com/grpc-ecosystem/go-grpc-middleware/releases/tag/v2.0.0-rc.2

Thanks. Would you mind also creating a providers/zerolog/v2.0.0-rc.2 tag so that the zerolog portion of v2 is "released"? Also, if there were changes to the other log providers, individual tags for those would be a good idea IMHO.

@irridia
Copy link
Contributor

irridia commented Jun 22, 2020

Hey @irridia! Can you share how did you correctly install v2 branch?

Importing at the github.com/grpc-ecosystem/go-grpc-middleware/v2 path should trigger go mod tidy to see the "/v2" suffix and pull the latest tag from the repo of the form v2.*. In this case, that will be v2.0.0-rc.2.

Generally I just delete the line from go.mod, and run go mod tidy to re-perform the dependency match. Though sometimes I need to go clean -modcache.

The same method applies when separately importing the log providers at github.com/grpc-ecosystem/go-grpc-middleware/providers/zerolog/v2. Right now there are no latest tags for zerolog, so to import it (for now) you'd have to find the actual commit hash of the latest v2 merge (880c1749170035e5a54794546dbfe0d45d10a11d in this case) and add it manually to go.mod.

As an example, from a fresh go mod init ; go mod tidy, tidy will error out but it creates a go.mod file with this line:

  github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-rc.2

Then to get zerolog (for example) importing, manually add this line to go mod:

  github.com/grpc-ecosystem/go-grpc-middleware/providers/zerolog/v2 880c1749170035e5a54794546dbfe0d45d10a11d

Once you run go mod tidy, go.mod will be rewritten as:

  github.com/grpc-ecosystem/go-grpc-middleware/providers/zerolog/v2 v2.0.0-rc.0.0.20200618173137-880c17491700
  github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-rc.2

And cheat mode is just "add those two lines to your go.mod". ;-)

@yashrsharma44
Copy link
Collaborator

yashrsharma44 commented Jun 22, 2020

Thank you so much @irridia ! This was a pretty cool. I googled a lot to find similar issues, but I didn't get any. But the explanation was really awesome, thanks!

Edit :
I wonder why go get doesn't work in this case, seems weird

@johanbrandhorst
Copy link
Collaborator

I've created a new release: https://github.com/grpc-ecosystem/go-grpc-middleware/releases/tag/v2.0.0-rc.2

Thanks. Would you mind also creating a providers/zerolog/v2.0.0-rc.2 tag so that the zerolog portion of v2 is "released"? Also, if there were changes to the other log providers, individual tags for those would be a good idea IMHO.

Done https://github.com/grpc-ecosystem/go-grpc-middleware/releases/tag/providers%2Fzerolog%2Fv2.0.0-rc.2. I have only tagged zerolog.

@irridia
Copy link
Contributor

irridia commented Jun 23, 2020

Thanks! go mod init ; go mod tidy now does everything automatically with the .../v2 imports. I'll start rolling v2 into our services and see how it goes.

	github.com/grpc-ecosystem/go-grpc-middleware/providers/zerolog/v2 v2.0.0-rc.2
	github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-rc.2

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.

None yet

5 participants