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

Adopt Docs Starter Pack #429

Merged
merged 3 commits into from
Jan 8, 2024
Merged

Conversation

rkratky
Copy link
Contributor

@rkratky rkratky commented Dec 14, 2023

Description

  • Exclude changes in 'doc/' dir from code CI checks.
  • Adopt Docs Starter Pack:
    • Add CI linkcheck
    • Substitute spellcheck & incl. lang. check w/ solution from Starter Pack
    • Fix ton of spelling mistakes
    • Update & fix markup for consistency
    • Use redirects instead of symlinks
  • Fix Doxygen config. & remove Doxyfile.

Checklist

  • Runs make check successfully.

  • Retains 100% code coverage (make check-coverage).

  • N/A: New/changed keys in YAML format are documented.

  • N/A: (Optional) Adds example YAML for new feature.

  • (Optional) Closes an open bug in Launchpad. -- Fixes FR-6115, FR-6157

@rkratky rkratky added enhancement New feature or request documentation Documentation improvements. labels Dec 14, 2023
@rkratky rkratky requested review from slyon and daniloegea December 14, 2023 18:05
@rkratky rkratky force-pushed the FR-6115_starter-pack branch 2 times, most recently from 7496a64 to 9dd4239 Compare December 14, 2023 18:26
@rkratky rkratky changed the title Adopt Docs Starter Pack [WIP] Adopt Docs Starter Pack Dec 14, 2023
@rkratky rkratky force-pushed the FR-6115_starter-pack branch 7 times, most recently from 28c1a52 to 33556d7 Compare December 14, 2023 23:10
@rkratky rkratky changed the title [WIP] Adopt Docs Starter Pack Adopt Docs Starter Pack Dec 14, 2023
@rkratky rkratky mentioned this pull request Dec 15, 2023
5 tasks
doc/Makefile Outdated Show resolved Hide resolved
doc/netplan-get.md Outdated Show resolved Hide resolved
Doxyfile Show resolved Hide resolved
Copy link
Collaborator

@daniloegea daniloegea left a comment

Choose a reason for hiding this comment

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

Thanks, Robert. It looks good. I left some minor comments. Let's see what Lukas says.

@rkratky rkratky force-pushed the FR-6115_starter-pack branch 4 times, most recently from 3efa157 to 720021e Compare January 3, 2024 11:09
Copy link
Collaborator

@slyon slyon left a comment

Choose a reason for hiding this comment

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

Thank you Robert! This is looking mostly good to me. I left a few comments & questions inline.

.github/workflows/automatic-doc-checks.yml Show resolved Hide resolved
doc/.sphinx/.wordlist.dic Outdated Show resolved Hide resolved
doc/.sphinx/_static/favicon.png Outdated Show resolved Hide resolved
doc/.sphinx/warnings.txt Outdated Show resolved Hide resolved
doc/Makefile Outdated Show resolved Hide resolved
doc/manpage-header.md Show resolved Hide resolved
doc/netplan-tutorial.md Outdated Show resolved Hide resolved
doc/netplan-yaml.md Outdated Show resolved Hide resolved
doc/structure-id.md Outdated Show resolved Hide resolved
@rkratky rkratky force-pushed the FR-6115_starter-pack branch from 720021e to 9090a88 Compare January 5, 2024 14:25
@rkratky
Copy link
Contributor Author

rkratky commented Jan 5, 2024

@slyon I addressed all the comments, and I believe this is good top merge. Thanks for the review.

@slyon
Copy link
Collaborator

slyon commented Jan 8, 2024

I only have one question/concern before merging, about the woke implementation.
AFAICT the new woke check only checks the docs inside doc/, whereas our old implementation (#303) actually checked all of the source code (i.e. public headers/API, ...). Maybe we should just keep that original check around, additionally? I.e. not removing this: https://github.com/canonical/netplan/pull/429/files#diff-651a780d1a032042f88eece53425bb26377cd39c299fa9dafbd643b126d6981f

WDYT? Would this work?

@rkratky rkratky force-pushed the FR-6115_starter-pack branch from 9090a88 to e01451c Compare January 8, 2024 14:02
- Add CI linkcheck
- Substitute spellcheck & incl. lang. check w/ solution
      from Starter Pack
- Fix ton of spelling mistakes
- Update & fix markup for consistency
- Use redirects instead of symlinks
@rkratky rkratky force-pushed the FR-6115_starter-pack branch from e01451c to e208814 Compare January 8, 2024 15:20
Copy link
Collaborator

@slyon slyon left a comment

Choose a reason for hiding this comment

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

Thanks Robert, for your contribution! With all the fixes landed, this LGTM!

@slyon slyon merged commit d3a408c into canonical:main Jan 8, 2024
14 of 16 checks passed
@rkratky rkratky deleted the FR-6115_starter-pack branch January 8, 2024 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Documentation improvements. enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants