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.

Sign up for GitHub

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

Jump to bottom

doc: new sidewalk setup doc review #694

Merged
merged 1 commit into from
Mar 24, 2025
Merged

doc: new sidewalk setup doc review #694

merged 1 commit into from
Mar 24, 2025

Conversation

annwoj
Copy link
Contributor

@annwoj annwoj commented Mar 21, 2025

Review of the PR687
Added an extension for easy copy-pasting of code blocks.

CI parameters

Jenkins:
  test-sdk-sidewalk: master
  # To reconfigure functional tests:
  # use GH labels func-* (default is func-commit)
  # or
  # Use YAML Filters. Helper side to set filters: https://tester-pc.nordicsemi.no:8080/test_mgmt
  # Filters (place copied YAML filters here):
  #  - filter1:
  #     board: nrf52

Description

JIRA ticket:

Self review

  • There is no commented code.
  • There are no TODO/FIXME comments without associated issue ticket.
  • Commits are properly organized.
  • Change has been tested.
  • Tests were updated (if applicable).

@github-actions github-actions bot added doc-required PR must not be merged without tech writer approval. source PR changing src files labels Mar 21, 2025
@annwoj annwoj force-pushed the migration_guide_doc_review branch from 5d7f327 to 48f5b31 Compare March 21, 2025 16:38
@annwoj annwoj mentioned this pull request Mar 21, 2025
Merged
5 tasks
@annwoj annwoj requested review from RobertGalatNordic and ktaborowski and removed request for RobertGalatNordic March 21, 2025 16:39
@github-actions GitHub Actions
Copy link

Sample diff used total

Memory usage did not change for any of the samples.

ktaborowski
ktaborowski approved these changes Mar 24, 2025
View reviewed changes
Copy link
Contributor

@ktaborowski ktaborowski left a comment

Choose a reason for hiding this comment

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

lgtm

ktaborowski
ktaborowski reviewed Mar 24, 2025
View reviewed changes
doc/releases_and_migration.rst Outdated
releases_and_migration/migration_guide_v280.rst
releases_and_migration/migration_guide_v260.rst
releases_and_migration/migration_guide_addon_v010.rst

Copy link
Contributor

@ktaborowski ktaborowski Mar 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

CI failure: "Please remove blank lines at end of 'doc/releases_and_migration.rst'"
afaik it requires precisely one newline at the end of the file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, was just waiting for any other suggestions before fixing the compliance :)

@totyz
Copy link
Contributor

totyz commented Mar 24, 2025

doc: https://jenkins-ncs.nordicsemi.no/job/KRKNG/job/Sidewalk/job/test-sdk-sidewalk/job/master/1629/artifact/html_doc/index.html

annwoj
annwoj commented Mar 24, 2025
View reviewed changes
doc/releases_and_migration/migration_guide_addon_v010.rst
@@ -1,101 +1,98 @@
.. _migration_guide_addon_v010:

Migration Guide for moving to Sidewalk Add-on
############################################
Migration guide for moving to Sidewalk Add-on
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ktaborowski should we add a release number here? The one that will be used for add-on? So it would be:

Migration guide for moving to Sidewalk Add-on (release v1.0.0)

not sure if the number is correct

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's better with no version - w plan to made a v1.0.0 add on version on next stable ncs, and the instruction should be valid there as well. Also there might be more v0.X.Y tags before for testing purposes

@annwoj
doc: new sidewalk setup doc review
ddb3819 
Review of the PR687
Added an extension for easy copy-pasting of code blocks.

Signed-off-by: Anna Wojdylo <anna.wojdylo@nordicsemi.no>
@annwoj annwoj force-pushed the migration_guide_doc_review branch from 48f5b31 to ddb3819 Compare March 24, 2025 10:34
@annwoj annwoj merged commit e342075 into nrfconnect:main Mar 24, 2025
63 of 64 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ktaborowski ktaborowski ktaborowski approved these changes

@totyz totyz totyz approved these changes

@RobertGalatNordic RobertGalatNordic Awaiting requested review from RobertGalatNordic

Assignees
No one assigned
Labels
doc-required PR must not be merged without tech writer approval. source PR changing src files
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@annwoj @totyz @ktaborowski