Skip to content

Windows top level build instructions#126

Merged
JShep1 merged 9 commits intomasterfrom
jshep1/top_level_windows_install
Jan 21, 2021
Merged

Windows top level build instructions#126
JShep1 merged 9 commits intomasterfrom
jshep1/top_level_windows_install

Conversation

@JShep1
Copy link
Copy Markdown

@JShep1 JShep1 commented Jan 6, 2021 •
edited
Loading

Signed-off-by: John Shepherd john@openrobotics.org

Fixes #96

add windows citadel build
ce5d3e8 
Signed-off-by: John Shepherd <john@openrobotics.org>
@github-actions github-actions bot added the 🏰 citadel Ignition Citadel label Jan 6, 2021
Remove windows notes
c98b7cc 
Signed-off-by: John Shepherd <john@openrobotics.org>
@chapulina chapulina added the documentation Improvements or additions to documentation label Jan 6, 2021
@chapulina chapulina mentioned this pull request Jan 7, 2021
Open
13 tasks
Minor fixes
2690dd9 
Signed-off-by: John Shepherd <john@openrobotics.org>
@JShep1 JShep1 marked this pull request as ready for review January 7, 2021 20:24
@JShep1 JShep1 requested a review from maryaB-osr as a code owner January 7, 2021 20:24
@JShep1
Copy link
Copy Markdown
Author

JShep1 commented Jan 7, 2021

Marking ready for review. I made this guide generalizable to any version of ign-gazebo so it's an easy copy paste. Even though ign-gazebo isn't yet supported, I chose it as the package to build up to in the source build, I thought it would be more straightforward than specifying all of the children dependencies of ign-gazebo

I was wondering if we now want to make an effort to make Windows versions of all the tutorials in all of the separate repos. Such as https://github.com/ignitionrobotics/ign-gui/blob/ign-gui4/tutorials/06_example.md

@JShep1 JShep1 requested review from chapulina and mabelzhang January 7, 2021 20:30
fixes
acde48b 
Signed-off-by: John Shepherd <john@openrobotics.org>
@JShep1 JShep1 linked an issue Jan 7, 2021 that may be closed by this pull request
Top-level Windows installation instructions #96
Closed
chapulina
chapulina reviewed Jan 14, 2021
View reviewed changes
Copy link
Copy Markdown
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.

I haven't tried it on Windows, just have some high-level comments.

I was wondering if we now want to make an effort to make Windows versions of all the tutorials in all of the separate repos

That's what @mabelzhang has been doing on #117, right?

Comment thread citadel/install.md Outdated
Comment thread citadel/install_windows.md Outdated
@chapulina chapulina requested a review from j-rivero January 14, 2021 04:08
@mabelzhang
Copy link
Copy Markdown
Contributor

mabelzhang commented Jan 14, 2021 •
edited
Loading

That's what @mabelzhang has been doing on #117, right?

I think he might be referring to the OS-specific commands in the feature tutorials, not the installation tutorials. Like, are users on their own to figure out what the equivalent commands are on Windows for the different features, or do we want to be responsible for testing all of them and making sure they work on Windows.

@chapulina
Copy link
Copy Markdown
Contributor

chapulina commented Jan 14, 2021

I think he might be referring to the OS-specific commands in the feature tutorials,

Ah yes, got it! This is out of the scope for now, but something that needs to be done in the future.

mabelzhang
mabelzhang reviewed Jan 15, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@mabelzhang mabelzhang 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.

I haven't tried the colcon commands yet. Just formatting comments.

Is there a way to preview these locally? I didn't find a CMake file. Is it Doxygen or something else?

Comment thread citadel/install_windows_src.md Outdated
Comment thread citadel/install_windows_src.md Outdated
mabelzhang
mabelzhang reviewed Jan 15, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@mabelzhang mabelzhang left a comment

Choose a reason for hiding this comment

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

I ran the tutorial. Compiling will take a while. Just a few comments on details.

Comment thread citadel/install_windows.md Outdated
Comment thread citadel/install_windows_src.md Outdated
Comment thread citadel/install_windows_src.md Outdated
Comment thread citadel/install_windows_src.md Outdated
Comment thread citadel/install_windows_src.md Outdated
Comment thread citadel/install_windows_src.md
Comment thread citadel/install_windows_src.md Outdated
Comment thread citadel/install_windows_src.md Outdated
John Shepherd added 3 commits January 15, 2021 14:54
suggested fixes
72f9eba 
Signed-off-by: John Shepherd <john@openrobotics.org>
Merge branch 'jshep1/top_level_windows_install' of https://github.com…
ee864c5 
…/ignitionrobotics/docs into jshep1/top_level_windows_install

Signed-off-by: John Shepherd <john@openrobotics.org>
Spacing and phrasing
6e4b398 
Signed-off-by: John Shepherd <john@openrobotics.org>
mabelzhang
mabelzhang approved these changes Jan 16, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@mabelzhang mabelzhang 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 addressing the comments! Colcon built for me as described. Looks good. Approving for content. Feel free to adjust formatting as needed.
@chapulina is there a way to preview these locally like we can with the Doxygen docs? The build_docs.sh seems to upload straight to the web.

@chapulina
Copy link
Copy Markdown
Contributor

chapulina commented Jan 19, 2021

is there a way to preview these locally like we can with the Doxygen docs?

No 😢 #85

@mabelzhang
Copy link
Copy Markdown
Contributor

mabelzhang commented Jan 19, 2021

Perfect description 😅
Alright then I have no more comments. We won't know if that Getting Started link is fixed till it's deployed.

chapulina
chapulina approved these changes Jan 20, 2021
View reviewed changes
Copy link
Copy Markdown
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.

I just have some nitpicks. Let's merge and deploy to see what it looks like!

Comment thread citadel/install.md Outdated
Comment thread citadel/install.md Outdated
Comment thread citadel/install_windows_src.md Outdated
Comment thread citadel/install_windows_src.md
nits
3618896 
Signed-off-by: John Shepherd <john@openrobotics.org>
@JShep1 JShep1 merged commit 9f3ebb9 into master Jan 21, 2021
@JShep1 JShep1 deleted the jshep1/top_level_windows_install branch January 21, 2021 01:51
@chapulina chapulina mentioned this pull request Mar 10, 2021
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mabelzhang mabelzhang mabelzhang approved these changes

@maryaB-osr maryaB-osr Awaiting requested review from maryaB-osr

@j-rivero j-rivero Awaiting requested review from j-rivero

+1 more reviewer

@chapulina chapulina chapulina approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

🏰 citadel Ignition Citadel documentation Improvements or additions to documentation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Top-level Windows installation instructions

5 participants

@JShep1 @mabelzhang @chapulina @j-rivero @osrf-triage