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.

Sign up for GitHub

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

Jump to bottom

ci: add ci_coverage.yml, update README.md #80

Merged
merged 4 commits into from
Jun 3, 2022
Merged

ci: add ci_coverage.yml, update README.md #80

merged 4 commits into from
Jun 3, 2022

Conversation

ethanhanjiahao
Copy link
Contributor

@ethanhanjiahao ethanhanjiahao commented May 30, 2022 •
edited
Loading

Signed-off-by: ethanhanjiahao xidianhanjiahao@gmail.com
issue: #74

@ethanhanjiahao ethanhanjiahao changed the title ci: add code coverage to envoyproxy/gateway project ci: add ci_coverage.yml, update README.md May 30, 2022
@ethanhanjiahao
ci: add ci_coverage.yml to workflow
1e42c8a 
Signed-off-by: ethanhanjiahao <xidianhanjiahao@gmail.com>
@ethanhanjiahao
ci: add ci_coverage.yml to workflow, update README.md
2ff2095 
Signed-off-by: ethanhanjiahao <xidianhanjiahao@gmail.com>
@youngnick youngnick mentioned this pull request May 30, 2022
Closed
youngnick
youngnick previously approved these changes May 31, 2022
View reviewed changes
Copy link
Contributor

@youngnick youngnick left a comment

Choose a reason for hiding this comment

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

LGTM

arkodg
arkodg previously approved these changes Jun 1, 2022
View reviewed changes
skriss
skriss reviewed Jun 1, 2022
View reviewed changes
Copy link
Contributor

@skriss skriss left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @ethanhanjiahao, just one question on organization.

.github/workflows/ci_coverage.yml Outdated Show resolved Hide resolved
@ethanhanjiahao ethanhanjiahao dismissed stale reviews from arkodg and youngnick via 3b01433 June 2, 2022 23:51
@ethanhanjiahao
move coverage to build_and_test.yaml test job
7cc9c1a 
Signed-off-by: ethanhanjiahao <xidianhanjiahao@163.com>
@ethanhanjiahao
integrate codecov action to workflow
e9a300b 
Signed-off-by: ethanhanjiahao <xidianhanjiahao@163.com>
arkodg
arkodg approved these changes Jun 3, 2022
View reviewed changes
Copy link
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

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

LGTM, thanks !
nit: you could also reuse make test with GO_TEST_EXTRA_ARGS=-coverprofile=coverage.xml -covermode=atomic make test

@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (main@f08f90b). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff            @@
##             main       #80   +/-   ##
========================================
  Coverage        ?   100.00%           
========================================
  Files           ?         1           
  Lines           ?         5           
  Branches        ?         0           
========================================
  Hits            ?         5           
  Misses          ?         0           
  Partials        ?         0

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 f08f90b...e9a300b. Read the comment docs.

skriss
skriss approved these changes Jun 3, 2022
View reviewed changes
Copy link
Contributor

@skriss skriss left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM, agree with @arkodg's suggestion about reusing the make target but not a big deal and we can update later if/when more flags/args are added. Thanks @ethanhanjiahao!

@skriss skriss merged commit bc2c27f into envoyproxy:main Jun 3, 2022
@skriss
Copy link
Contributor

skriss commented Jun 3, 2022

Note, I left #74 open until we add PR checks that enforce required code coverage #'s.

@danehans
Copy link
Contributor

danehans commented Jun 8, 2022

xref #63

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shaneutt shaneutt shaneutt left review comments

@skriss skriss skriss approved these changes

@arkodg arkodg arkodg approved these changes

@youngnick youngnick Awaiting requested review from youngnick

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@ethanhanjiahao @codecov-commenter @skriss @danehans @shaneutt @youngnick @arkodg