Skip to content

Adding information to the LabVIEW_Debugging_Techniques.md #9

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

Merged
merged 22 commits into from
Sep 29, 2021

Conversation

cchidean
Copy link
Contributor

@cchidean cchidean commented Sep 3, 2021

What does this Pull Request accomplish?

The Pull Request adds information about Custom Devices to the LabVIEW_Debugging_Techniques.md file

Why should this Pull Request be merged?

Because the information in the file is part of the Custom Devices Handbook documentation.

What testing has been done?

https://niveristand-custom-device-handbook--9.org.readthedocs.build/en/9/LabVIEW_Debugging_Techniques.html

Copy link
Collaborator

@PaulDanH PaulDanH left a comment

Choose a reason for hiding this comment

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

Hey Calin, please check my above suggestions and comments. Thanks!

Copy link

@nidaring nidaring left a comment

Choose a reason for hiding this comment

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

Overall, I'm not sure what purpose this document serves vs the table here: https://www.ni.com/documentation/en/veristand/latest/manual/custom-device-benchmark-debug/

I'm guessing that it is meant to be a 'companion document' that adds more color to the tech comms help? If so, then maybe we should have the same ordering of 'tools' and make sure the same tools show up in both places?

@WillCawley
Copy link
Contributor

Hey @cchidean. Can you remove me from this review? I'll submit a separate pull-request with updates for clarity and concision when you all are done with making sure it is technically correct.

@PaulDanH PaulDanH removed the request for review from WillCawley September 13, 2021 19:19
@PaulDanH
Copy link
Collaborator

PaulDanH commented Sep 13, 2021

Hey @cchidean. Can you remove me from this review? I'll submit a separate pull-request with updates for clarity and concision when you all are done with making sure it is technically correct.

Hey @WillCawley you can always use insert a suggestion button for small changes that can be incorporated in the same PR. This worked great for us on the first pass review.

@cchidean
Copy link
Contributor Author

Overall, I'm not sure what purpose this document serves vs the table here: https://www.ni.com/documentation/en/veristand/latest/manual/custom-device-benchmark-debug/

I'm guessing that it is meant to be a 'companion document' that adds more color to the tech comms help? If so, then maybe we should have the same ordering of 'tools' and make sure the same tools show up in both places?

@cheloizaguirre what do you think about this question?

@cheloizaguirre
Copy link

Overall, I'm not sure what purpose this document serves vs the table here: https://www.ni.com/documentation/en/veristand/latest/manual/custom-device-benchmark-debug/
I'm guessing that it is meant to be a 'companion document' that adds more color to the tech comms help? If so, then maybe we should have the same ordering of 'tools' and make sure the same tools show up in both places?

@cheloizaguirre what do you think about this question?
This document seems slightly better than the one in the manual, which makes sense because this guide should be more detailed. We can tweak it later, it seems good enough to survive the first pass

Copy link

@cheloizaguirre cheloizaguirre left a comment

Choose a reason for hiding this comment

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

Overall it's good enough for me. Just do whatever Paul says for now and we can look into improving other content later.

cchidean and others added 3 commits September 28, 2021 23:35
implemented feedback

Co-authored-by: Paul Huțanu <62251693+phutanu@users.noreply.github.com>
@cchidean cchidean requested a review from PaulDanH September 28, 2021 20:44
Copy link
Collaborator

@PaulDanH PaulDanH left a comment

Choose a reason for hiding this comment

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

Small rendering changes for consistency. Please check and submit these, and this one is good to go. Thanks!

implemented feedback

Co-authored-by: Paul Huțanu <62251693+phutanu@users.noreply.github.com>
@cchidean cchidean requested a review from PaulDanH September 29, 2021 09:38
@PaulDanH PaulDanH self-requested a review September 29, 2021 11:00
@PaulDanH
Copy link
Collaborator

Please delete "docs/images/Disable_ni_emb.jpg" as it is not used anymore.

And this is one is good to go.

@PaulDanH PaulDanH merged commit c371000 into ni:main Sep 29, 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.

6 participants