-
-
Notifications
You must be signed in to change notification settings - Fork 477
Correct example sketch names #1643
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
Conversation
content/hardware/06.nicla/boards/nicla-sense-me/tutorials/connecting-to-iot-cloud/content.md
Outdated
Show resolved
Hide resolved
To me, it looks good, but the approval review should come from the maker content and pro content people, I've also tagged @jcarolinares here since there was one example from Opta ⭐
Thank you, @per1234!
Looks good from Maker side ✅
@jacobhylen would you mind providing an approval via the GitHub pull request review system?
The repository is configured to require PR approval via the review system so this is required from a technical standpoint even though an approving comment is equivalent from a human perspective.
@per1234 we are waiting for the review of @jcarolinares before we merge, I think maybe that's why @jacobhylen didn't approve through the system :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello,
First of all thank you for the PR @per1234 , really appreciated.
Please take a look at my review, I suggested some changes and generated a jira ticket for one of them (I suggest to simple remove that change, we will take care of it in the task)
Please take a look and don't hesitate to contact me in case you need it! Thanks!
content/hardware/04.pro/boards/portenta-h7/tutorials/flash-optimized-key-value-store/content.md
Show resolved
Hide resolved
content/hardware/04.pro/boards/portenta-h7/tutorials/over-the-air-update/content.md
Show resolved
Hide resolved
content/hardware/04.pro/boards/portenta-h7/tutorials/wifi-access-point/content.md
Show resolved
Hide resolved
...ardware/04.pro/shields/portenta-vision-shield/tutorials/speech-recognition-engine/content.md
Show resolved
Hide resolved
...ardware/04.pro/shields/portenta-vision-shield/tutorials/speech-recognition-engine/content.md
Outdated
Show resolved
Hide resolved
content/hardware/06.nicla/boards/nicla-sense-me/tutorials/connecting-to-iot-cloud/content.md
Show resolved
Hide resolved
content/hardware/06.nicla/boards/nicla-sense-me/tutorials/user-manual/content.md
Show resolved
Hide resolved
content/hardware/06.nicla/boards/nicla-vision/tutorials/proximity/content.md
Show resolved
Hide resolved
content/hardware/06.nicla/boards/nicla-vision/tutorials/user-manual/content.md
Show resolved
Hide resolved
...ent/hardware/07.opta/opta-family/opta/tutorials/getting-started-with-aws-iot-core/content.md
Show resolved
Hide resolved
fadd6c8 to
ad2b96e
Compare
@per1234 it seems like there are some conflicts that need to be resolved before we can review and approve this PR, can you take care of them? 🙏🏼
And then we are also waiting for @jcarolinares's approval 🙏🏼
it seems like there are some conflicts that need to be resolved before we can review and approve this PR, can you take care of them?
Yes, I will take care of them, but I will only do it after I get an approval because at the rate this thing is moving it would only be a waste of my time to do it now since there will likely just be more conflicts by the time I manage to obtain an approval.
it seems like there are some conflicts that need to be resolved before we can review and approve this PR, can you take care of them?
Yes, I will take care of them, but I will only do it after I get an approval because at the rate this thing is moving it would only be a waste of my time to do it now since there will likely just be more conflicts by the time I manage to obtain an approval.
Hello @per1234
Let's put the following statement clear:
I'm not going to approve a pull request without having the conflicts solved, so I can see what are the changes that will be finally applied to the actual documentation.
It is not a good practice in software development, and it is not a good practice here.
"To not waste your time" I've tried to help you solve the conflicts by myself, but I cannot do it because the branch of the pull request comes from a fork of this repo inside your personal github https://github.com/per1234/docs-content
That is ok and I get why are you doing it, but it is just not necessary and complicates things without need. Please clone and commit to this repo next time, you should have permissions, in case you don't contact me and Josephine to give you the right permissions to it, so we can jump to the branches and help you change things that are important for the review in case we need.
This review is finished but not approved until we fix the conflicts, once you have it please ping me or @jhansson-ard to approve and merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This review is finished but not approved until we fix the conflicts, once you have it please ping me or @jhansson-ard to approve and merge. Please read full explanation from above comment
The library was incorrectly referred to as "TinyGL". This would make it difficult for the user to follow the tutorial, including finding the library in Library Manager and the example sketches once the library was installed.
The content refers to various example sketches that can be accessed via the Arduino development tool interfaces. Some of these references used incorrect names for the example sketches or their parent menu paths.
Co-authored-by: Josefine Hansson <66409231+jhansson-ard@users.noreply.github.com>
...raries There is a separate speech recognition library for each of the supported boards. The library name all use the same "DSpotterSDK_Maker_" prefix, differentiated with a suffix to indicate the target board. The tutorial contains instructions for opening the library's example sketch. These instructions are intended to be applicable for use with any of the library variants so it is necessary for the reader to extrapolate the correct library name. The new approach is to use a placeholder to indicate the board name suffix on the library name. The common convention of indicating a placeholder via angle brackets was chosen. This causes a visual collision with the tutorials use of right angle brackets as separators when indicating menu path navigation, as is the standard in Arduino documentation. In order to avoid this collision, the tutorial is migrated to using the alternative "->" menu path separator.
28ffb82 to
aa7ed7c
Compare
What This PR Changes
The content refers to various example sketches that can be accessed via the Arduino development tool interfaces. Some of these references used incorrect names for the example sketches or their parent menu paths.
References:
Contribution Guidelines