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

Port codecov to new configuration #170

Merged
merged 2 commits into from
May 14, 2021
Merged

Conversation

j-rivero
Copy link
Contributor

@j-rivero j-rivero commented May 6, 2021

See gazebo-tooling/action-gz-ci#32

Signed-off-by: Jose Luis Rivero [email protected]

@j-rivero j-rivero requested review from adlarkin and mxgrey as code owners May 6, 2021 13:39
@github-actions github-actions bot added 🏢 edifice Ignition Edifice 🏯 fortress Ignition Fortress 🏰 citadel Ignition Citadel 🔮 dome Ignition Dome labels May 6, 2021
@adlarkin
Copy link
Contributor

adlarkin commented May 6, 2021

Would it be worth merging this and then forward porting it to main instead of having a separate PR (#169) for main?

@adlarkin
Copy link
Contributor

adlarkin commented May 6, 2021

I'm also seeing a failure in GitHub actions:

make: *** No rule to make target 'coverage'.  Stop.

@adlarkin
Copy link
Contributor

@j-rivero friendly ping

@j-rivero
Copy link
Contributor Author

@j-rivero friendly ping

thanks! I've disabled codecov, if I'm not wrong there is no support in this repository for it

Copy link
Contributor

@adlarkin adlarkin left a comment

Choose a reason for hiding this comment

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

I've disabled codecov, if I'm not wrong there is no support in this repository for it

All checks pass, so I'm approving this 👍 @scpeters or @chapulina, beofre we merge this, can one of you confirm that disabling codecov in this repository is okay?

@chapulina
Copy link
Contributor

disabling codecov in this repository is okay?

Yup, I think it should be ok because we aren't generating coverage data either for the tests in examples nor the tests in test. Not sure if there would be an easy way of adding coverage for CMake files, and I think we shouldn't bother with the utilities C++ code since that's being moved to ign-utils.

@adlarkin
Copy link
Contributor

Sounds good 👍 merging!

@adlarkin adlarkin merged commit 2f69ac3 into ign-cmake2 May 14, 2021
@adlarkin adlarkin deleted the codecov_port_ign-cmake2 branch May 14, 2021 18:20
This was referenced May 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏰 citadel Ignition Citadel 🔮 dome Ignition Dome 🏢 edifice Ignition Edifice 🏯 fortress Ignition Fortress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants