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

Updates to developer_guide for clarity. #96

Merged
19 commits merged into from
May 23, 2022

Conversation

lobotmcj
Copy link
Contributor

@lobotmcj lobotmcj commented May 8, 2022

Closes #95

@mdemoret-nv mdemoret-nv added non-breaking Non-breaking change doc Improvements or additions to documentation labels May 10, 2022
@mdemoret-nv mdemoret-nv added this to PR - WIP in v22.06 Release via automation May 11, 2022
@mdemoret-nv
Copy link
Contributor

@dagardner-nv Can you please review these documentation changes?

Copy link
Contributor Author

@lobotmcj lobotmcj left a comment

Choose a reason for hiding this comment

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

made a few corrections and comments to aid in review.

@lobotmcj
Copy link
Contributor Author

(if preferred, I can also bundle up all of the comment suggestions and squash them into the original commit, rebase, and force push to my branch so the PR will just be one commit with everything)

Copy link
Contributor

@pdmack pdmack left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @lobotmcj

@lobotmcj
Copy link
Contributor Author

@dagardner-nv thank you for reviewing. Apparently GitHub adds one additional commit per accepted suggestion in the PR review process. This is going to make for a lot of commits. But it will be fine -- to keep things clean, once the review is all done and all the commits are added, I will rebase and squash so that it's just one linear commit to the Morpheus git history. Let me know if this is acceptable to you.

v22.06 Release automation moved this from PR - WIP to PR - Needs Review May 18, 2022
Copy link
Contributor Author

@lobotmcj lobotmcj left a comment

Choose a reason for hiding this comment

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

Sorry if this blasted you with notifications, still getting used to how GitHub does this vice GitLab.

@dagardner-nv
Copy link
Contributor

@lobotmcj don't worry about rebasing and squashing I believe that is something that we do on merge.

Copy link
Contributor Author

@lobotmcj lobotmcj left a comment

Choose a reason for hiding this comment

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

consistently using unhyphenated "pass through" in all sentences per request

docs/source/developer_guide/architecture.md Outdated Show resolved Hide resolved
docs/source/developer_guide/architecture.md Outdated Show resolved Hide resolved
docs/source/developer_guide/guides/3_simple_cpp_stage.md Outdated Show resolved Hide resolved
docs/source/developer_guide/guides/3_simple_cpp_stage.md Outdated Show resolved Hide resolved
Copy link
Contributor Author

@lobotmcj lobotmcj left a comment

Choose a reason for hiding this comment

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

x2 "pass through" instead of "pass-through" changes.

@dagardner-nv
Copy link
Contributor

LGTM, thanks

@pdmack pdmack requested a review from a team as a code owner May 23, 2022 21:47
@pdmack
Copy link
Contributor

pdmack commented May 23, 2022

@lobotmcj could you please rebase to pick up #80 ?
Apologies, still early days as we sort our CI.

@lobotmcj
Copy link
Contributor Author

@lobotmcj could you please rebase to pick up #80 ? Apologies, still early days as we sort our CI.

sure no problem

dagardner-nv and others added 15 commits May 23, 2022 17:58
Co-authored-by: Jeffrey James <lobotmcj@gmail.com>
Co-authored-by: Jeffrey James <lobotmcj@gmail.com>
Co-authored-by: Jeffrey James <lobotmcj@gmail.com>
Co-authored-by: Jeffrey James <lobotmcj@gmail.com>
Co-authored-by: Jeffrey James <lobotmcj@gmail.com>
Co-authored-by: Jeffrey James <lobotmcj@gmail.com>
Co-authored-by: Jeffrey James <lobotmcj@gmail.com>
Co-authored-by: Jeffrey James <lobotmcj@gmail.com>
Co-authored-by: Jeffrey James <lobotmcj@gmail.com>
Co-authored-by: Jeffrey James <lobotmcj@gmail.com>
Co-authored-by: Jeffrey James <lobotmcj@gmail.com>
@lobotmcj
Copy link
Contributor Author

@pdmack let me know if you also want to squash the commits together; I'm fine either way

@pdmack
Copy link
Contributor

pdmack commented May 23, 2022

@gpucibot merge

@ghost ghost merged commit d46382b into nv-morpheus:branch-22.06 May 23, 2022
v22.06 Release automation moved this from PR - Needs Review to Done May 23, 2022
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Improvements or additions to documentation non-breaking Non-breaking change
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

[DOC] Suggested updates to developer guide for clarity
4 participants