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 Windows installation; move installation in README to tutorial #146

Merged
merged 13 commits into from
Jan 21, 2021

Conversation

mabelzhang
Copy link
Contributor

@mabelzhang mabelzhang commented Dec 24, 2020

Partially addresses gazebosim/docs#117 and gazebosim/docs#14 (comment) .

@github-actions github-actions bot added 🏢 edifice Ignition Edifice 🏰 citadel Ignition Citadel 📜 blueprint Ignition Blueprint 🔮 dome Ignition Dome labels Dec 24, 2020
@mabelzhang mabelzhang added the Windows Windows support label Dec 24, 2020
@codecov
Copy link

codecov bot commented Dec 24, 2020

Codecov Report

Merging #146 (113c79b) into ign-common3 (af8a83f) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           ign-common3     #146   +/-   ##
============================================
  Coverage        75.53%   75.53%           
============================================
  Files               69       69           
  Lines             9623     9623           
============================================
  Hits              7269     7269           
  Misses            2354     2354           

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 af8a83f...113c79b. Read the comment docs.

Signed-off-by: Mabel Zhang <[email protected]>
@traversaro
Copy link
Contributor

Not sure if that makes the difference or I'm missing something.

The difference is that the ignition-cmake available in conda-forge contains a patch to fix the GTS-problem reported by @JShep1 :

So the problem is still present if ignition-cmake is compiled from source when building all the ignition projects together with colcon, but not present if ignition-cmake is installed via conda-forge.

Signed-off-by: Mabel Zhang <[email protected]>
@chapulina chapulina added the documentation Improvements or additions to documentation label Dec 28, 2020
tutorials/install.md Outdated Show resolved Hide resolved
Copy link

@JShep1 JShep1 left a comment

Choose a reason for hiding this comment

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

Small consistency nit but LGMT otherwise

@mabelzhang
Copy link
Contributor Author

mabelzhang commented Jan 8, 2021

Same comment about formatting as gazebosim/gz-math#185 (comment)
fb6323a tries to fix it.

I'm not sure if that fixes it though. Looking at the profiler tutorial in this repo, Troubleshoot section, bullets 2 and 4 have code blocks, one with a preceding empty line and one without, both rendered incorrectly. So the empty line isn't the answer. Do you think changing from 4 spaces to 2 spaces helps??? Notably, the {.sh} doesn't help either, because the ign-physics tutorial that renders correctly doesn't have that, these incorrect ones do.

@mabelzhang
Copy link
Contributor Author

Formatting has been fixed, given the tip in the other PR.
So yeah, 2 spaces made a difference!

Signed-off-by: Mabel Zhang <[email protected]>
Signed-off-by: Mabel Zhang <[email protected]>
Signed-off-by: John Shepherd <[email protected]>
@chapulina chapulina merged commit eff2887 into ign-common3 Jan 21, 2021
@chapulina chapulina deleted the mabelzhang/windows_install branch January 21, 2021 02:04
@peci1 peci1 mentioned this pull request Apr 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📜 blueprint Ignition Blueprint 🏰 citadel Ignition Citadel documentation Improvements or additions to documentation 🔮 dome Ignition Dome 🏢 edifice Ignition Edifice Windows Windows support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants