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

Windows tutorial tweaks #58

Merged
merged 2 commits into from
Jun 22, 2018
Merged

Windows tutorial tweaks #58

merged 2 commits into from
Jun 22, 2018

Conversation

sloretz
Copy link
Contributor

@sloretz sloretz commented Jun 22, 2018

  • set instead of export
  • use bat for cmd.exe highlighting
  • Remove some trailing backticks
  • Add note about RANDFILE
  • Remove -s option from curl (my installation of curl fails because it doesn't support https, and slient hides this)

`set` instead of `export`
use `bat` for cmd.exe highlighting
Remove trailing backticks
Add note about RANDFILE
@sloretz sloretz self-assigned this Jun 22, 2018
@dirk-thomas dirk-thomas added the in progress Actively being worked on (Kanban column) label Jun 22, 2018
@sloretz sloretz added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Jun 22, 2018
Copy link
Member

@mikaelarguedas mikaelarguedas 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 one inline question

```bash
curl -sk https://raw.githubusercontent.com/ros2/sros2/ardent/examples/sample_policy.yaml -o .\demo_keys\policies.yaml
```bat
curl -k https://raw.githubusercontent.com/ros2/sros2/ardent/examples/sample_policy.yaml -o .\demo_keys\policies.yaml
Copy link
Member

Choose a reason for hiding this comment

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

Is this because we want additional printing if the download fail?
Or did something else motivate this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my case -s was hiding an error saying curl was not built with https support, and I almost didn't notice due to #41. By chance I saw the file wasn't there when deleting the demo_keys directory for another RMW implementation.

Copy link
Member

Choose a reason for hiding this comment

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

👍 thanks for clarifying

@sloretz sloretz merged commit 54394af into master Jun 22, 2018
@dirk-thomas dirk-thomas removed the in review Waiting for review (Kanban column) label Jun 22, 2018
@sloretz sloretz deleted the windows_tutorial_tweaks branch June 22, 2018 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants