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

Examples: Add proper Table of Contents #20957

Merged
merged 5 commits into from
Nov 14, 2024

Conversation

AnnsAnns
Copy link
Contributor

@AnnsAnns AnnsAnns commented Nov 6, 2024

Contribution description

Currently there is no description of the examples (Some not even having a README).

This PR aims to add a basic table of contents README for the examples directory to improve the UX when searching for examples while also adding a few minor READMEs for undocumented examples.

It also tries to improve the general readability of the folder by categorizing examples into easier to digest categories within the README. This makes specific examples easier to find.

Testing procedure

There are no changes within this PR that would affect the code.

Issues/PRs references

This aims to help with some of the issues mentioned within #20388

@AnnsAnns AnnsAnns requested a review from jia200x as a code owner November 6, 2024 16:14
@github-actions github-actions bot added Area: doc Area: Documentation Area: examples Area: Example Applications labels Nov 6, 2024
@benpicco benpicco requested a review from Teufelchen1 November 6, 2024 16:52
@mguetschow mguetschow added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 7, 2024
@riot-ci
Copy link

riot-ci commented Nov 7, 2024

Murdock results

✔️ PASSED

6f19763 examples/readme: Add subfolders example to categories

Success Failures Total Runtime
1 0 1 01m:27s

Artifacts

Copy link
Contributor

@Teufelchen1 Teufelchen1 left a comment

Choose a reason for hiding this comment

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

Didn't have time to do a throughout review but already found little issues.

examples/README.md Outdated Show resolved Hide resolved
examples/README.md Outdated Show resolved Hide resolved
examples/README.md Outdated Show resolved Hide resolved
examples/README.md Outdated Show resolved Hide resolved
examples/README.md Outdated Show resolved Hide resolved
examples/pio_blink/README.md Outdated Show resolved Hide resolved
examples/telnet_server/README.md Show resolved Hide resolved
Copy link
Contributor Author

@AnnsAnns AnnsAnns left a comment

Choose a reason for hiding this comment

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

Will improve things mentioned, thank you for the review :)

examples/telnet_server/README.md Show resolved Hide resolved
@leandrolanzieri
Copy link
Contributor

@AnnsAnns do you think it would be possible to include a basic static test that goes over the directory list in examples and checks that at least they're mentioned in the README.md? I'm afraid this index can age if not enforced somehow...

Copy link
Contributor

@mguetschow mguetschow left a comment

Choose a reason for hiding this comment

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

Thanks for your work on this! Just had a brief look and left some comments below.

Could we maybe have a two-column table-style of the list? At least, I would argue for moving the explanation from the single subitem to the item itself, potentially after a colon (:).

examples/README.md Outdated Show resolved Hide resolved
examples/README.md Outdated Show resolved Hide resolved
examples/README.md Outdated Show resolved Hide resolved
examples/README.md Outdated Show resolved Hide resolved
examples/README.md Outdated Show resolved Hide resolved
examples/README.md Outdated Show resolved Hide resolved
examples/README.md Outdated Show resolved Hide resolved
examples/README.md Outdated Show resolved Hide resolved
examples/telnet_server/README.md Show resolved Hide resolved
examples/timer_periodic_wakeup/README:md Outdated Show resolved Hide resolved
Copy link
Contributor

@krzysztof-cabaj krzysztof-cabaj left a comment

Choose a reason for hiding this comment

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

I think that there is a little typo for timer_periodic_wakeup and this file should be named README.md not README:md (period instead of colon).

@AnnsAnns
Copy link
Contributor Author

@AnnsAnns do you think it would be possible to include a basic static test that goes over the directory list in examples and checks that at least they're mentioned in the README.md? I'm afraid this index can age if not enforced somehow...

Looking into it, should be feasible :)

@github-actions github-actions bot added the Area: tools Area: Supplementary tools label Nov 12, 2024
@AnnsAnns
Copy link
Contributor Author

@AnnsAnns do you think it would be possible to include a basic static test that goes over the directory list in examples and checks that at least they're mentioned in the README.md? I'm afraid this index can age if not enforced somehow...

Added (Even found a few missing entries thanks to it 😅)

@AnnsAnns
Copy link
Contributor Author

AnnsAnns commented Nov 12, 2024

I hope I covered all mentioned issues, let me know if there is anything else :)

I also changed it to a table format as mentioned by @mguetschow, it definitely looks better on github

@Teufelchen1
Copy link
Contributor

Nice! I am happy with it. There's one static test that you should fix. A nit pick could be to combine the two ci tests into one, reducing the burden on the servers but I believe that is not significant here.

Copy link
Contributor

@mguetschow mguetschow left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!

@mguetschow mguetschow added this pull request to the merge queue Nov 12, 2024
@mguetschow mguetschow removed this pull request from the merge queue due to a manual request Nov 12, 2024
@mguetschow
Copy link
Contributor

Would you mind squashing the commits together?

AnnsAnns and others added 2 commits November 12, 2024 16:17
examples: ToC about all examples

examples: Fix whiteline issues

examples: Improve wording of Rust example

Co-authored-by: Teufelchen <[email protected]>

examples: Reduce redundant wording

Co-authored-by: Teufelchen <[email protected]>

examples: remove mention of repository

Co-authored-by: Teufelchen <[email protected]>

examples: Improve wording of coap example text

Co-authored-by: Teufelchen <[email protected]>
examples/readme: fix SAUL spelling mistake

examples/readme: Link to README.md instead of folder

examples/readme: increase title size

examples/readme: make networking category name a bit clearer

examples/readme: move nimble to own category within BLE

examples/readme: move dtls to own category

examples/readme: move heart rate sensor to nimble category

examples/readme: move arduino sketch to languages

examples/timer_periodic: fix file name issue

examples/readme: change level wording about nanocoap
examples/readme: add missing entries

examples: document examples with no readme
@mguetschow
Copy link
Contributor

Did you maybe accidentally drop some commit?

./dist/tools/examples_check/check_has_readme.sh x
  Command output:
  
  	The following directories are missing a README:
  	- pio_blink
  
./dist/tools/examples_check/check_in_readme.sh x
  Command output:
  
  	The following directories are missing in the README.md file:
  	- sock_tcp_echo

examples: Remove whitespace to fix CI
@AnnsAnns
Copy link
Contributor Author

AnnsAnns commented Nov 12, 2024

Did you maybe accidentally drop some commit?

./dist/tools/examples_check/check_has_readme.sh x
  Command output:
  
  	The following directories are missing a README:
  	- pio_blink
  
./dist/tools/examples_check/check_in_readme.sh x
  Command output:
  
  	The following directories are missing in the README.md file:
  	- sock_tcp_echo

Fixed, sock_tcp_echo was a new example (and pio_blink went missing)

@mguetschow mguetschow added this pull request to the merge queue Nov 13, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 13, 2024
@mguetschow
Copy link
Contributor

@AnnsAnns we've merged a new example just today with #20024, and your CI check rightfully noticed that. Would you mind adding it to the table of content?

@AnnsAnns
Copy link
Contributor Author

@AnnsAnns we've merged a new example just today with #20024, and your CI check rightfully noticed that. Would you mind adding it to the table of content?

Done

@Teufelchen1 Teufelchen1 added this pull request to the merge queue Nov 13, 2024
Merged via the queue into RIOT-OS:master with commit 03b6122 Nov 14, 2024
25 checks passed
@Teufelchen1
Copy link
Contributor

Congrats! It's really awesome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: doc Area: Documentation Area: examples Area: Example Applications Area: tools Area: Supplementary tools CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants