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

Enable new CMake policy to fix protobuf compilation #1046

Merged
merged 3 commits into from
Sep 24, 2021

Conversation

Blast545
Copy link
Contributor

Signed-off-by: Jorge Perez [email protected]

🦟 Bug fix

Fixes #990

Summary

Discussion is provided in the original ticket.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

@Blast545 Blast545 added the Windows Windows support label Sep 23, 2021
@Blast545 Blast545 requested a review from chapulina as a code owner September 23, 2021 20:30
@github-actions github-actions bot added the 🏢 edifice Ignition Edifice label Sep 23, 2021
@Blast545 Blast545 added the bug Something isn't working label Sep 23, 2021
@Blast545 Blast545 changed the base branch from ign-gazebo5 to main September 23, 2021 20:33
@Blast545 Blast545 removed the 🏢 edifice Ignition Edifice label Sep 23, 2021
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

LGTM with happy CI.

@chapulina chapulina added the beta Targeting beta release of upcoming collection label Sep 23, 2021
@codecov
Copy link

codecov bot commented Sep 23, 2021

Codecov Report

Merging #1046 (b165657) into main (f4d47b5) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1046      +/-   ##
==========================================
- Coverage   63.71%   63.70%   -0.01%     
==========================================
  Files         257      257              
  Lines       20064    20064              
==========================================
- Hits        12783    12782       -1     
- Misses       7281     7282       +1     
Impacted Files Coverage Δ
src/EntityComponentManager.cc 84.95% <0.00%> (-0.15%) ⬇️

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 f4d47b5...b165657. Read the comment docs.

@Blast545
Copy link
Contributor Author

Both of these jobs run on top of this PR: https://build.osrfoundation.org/job/ign_gazebo-pr-win/3151/ and https://build.osrfoundation.org/job/ign_gazebo-pr-win/3154/ and neither have the Protobuf CMake error.

I think this is ready to merge and backport, I'll keep an eye to CI after this is merged as well.

@chapulina chapulina merged commit 1c0381d into main Sep 24, 2021
@chapulina chapulina deleted the blast545/fix_protobuf_cmake branch September 24, 2021 16:21
WilliamLewww pushed a commit to WilliamLewww/ign-gazebo that referenced this pull request Dec 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta Targeting beta release of upcoming collection bug Something isn't working Windows Windows support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

👩‍🌾 Windows build failures: Unknown CMake command "PROTOBUF_GENERATE_CPP".
2 participants