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

fix/doc-jetson-installation #9633

Merged
merged 7 commits into from
Oct 17, 2021
Merged

Conversation

tkazik
Copy link
Contributor

@tkazik tkazik commented Aug 15, 2021

This PR fixes the installation for jetson, which is currently broken for the second part. It also has some minor editorial changes to enhance readability.

Copy link
Collaborator

@ev-mp ev-mp left a comment

Choose a reason for hiding this comment

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

@tkazik , thank you for the contribution.
Can you restructure the PR so that the editorial and content changes will be captured in separate commits for review?
Thanks

@tkazik
Copy link
Contributor Author

tkazik commented Aug 16, 2021

Besides the fix, 99% of the changes in this PR consists of editorial changes in order to satisfy the markup rules...the diff looks like as if there are many content changes, but this is not the case. You can verify that using the markdown preview in e.g. VS Code.

@ev-mp
Copy link
Collaborator

ev-mp commented Aug 17, 2021

Well, so please just extract the fix into a separate commit for review.
Searching for a needle in a haystack in not an efficient way to review PRs :)

@tkazik
Copy link
Contributor Author

tkazik commented Aug 17, 2021

alright, so it is now split into 'fix' and 'editorial' changes.

Copy link
Collaborator

@ev-mp ev-mp left a comment

Choose a reason for hiding this comment

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

@tkazik , thank you for the quick response.
While I think the fix commit is ok the "editorial" change seem to break the layout and exposes the MD syntax, at least in chap#4 and below. See example
image

The first part seem ok,, so if you can refactor it into a separate PR it will be merged promptly.
Thanks


2. **Establish Developer's Environment**
Copy link
Collaborator

Choose a reason for hiding this comment

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

This line is a header. Put double-space or /n to separate from the para below

@ev-mp ev-mp changed the base branch from master to development August 17, 2021 19:15
@tkazik
Copy link
Contributor Author

tkazik commented Aug 20, 2021

hm, what exactly is jenkins complaining about?

Copy link
Collaborator

@ev-mp ev-mp left a comment

Choose a reason for hiding this comment

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

@tkazik , thank you for contribution!

@ev-mp ev-mp merged commit 047829b into IntelRealSense:development Oct 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants