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

Document and restructure libnetplan's public API symbols #438

Merged
merged 4 commits into from Jan 29, 2024

Conversation

slyon
Copy link
Collaborator

@slyon slyon commented Jan 24, 2024

Description

Please review commit-by-commit.

It's mostly adding Doxygen comments to our public functions.
But also dropping one public symbol from the API (netplan_generate), which is not properly implemented and still unused, so let's get rid of it until we grow a need for it and can have it implemented the correct way.

Finally, I'm moving around the documented declarations into their related header files:

  • parser.h / parser-nm.h: Functions related to NetplanParser*
  • state.h: Function related to NetplanState*
  • netdef.h: Functions related to NetplanNetDefinition*
  • utils.h: Function not related to any specific object
  • types.h: data structures, no functions

Note

You can ignore the final commit (abi-compat: Update for dropped 'netplan_generate' symbol), which is auto-generated noise to abi-compat/jammy_1.0.xml.

FR-6079

Checklist

  • Runs make check successfully.
  • Retains 100% code coverage (make check-coverage).
  • New/changed keys in YAML format are documented.
  • (Optional) Adds example YAML for new feature.
  • (Optional) Closes an open bug in Launchpad.

@slyon slyon changed the title Document libnetplan's public API symbols Document and restructure libnetplan's public API symbols Jan 24, 2024
@slyon slyon force-pushed the api-docs branch 3 times, most recently from a41ac1e to 4bff7e6 Compare January 24, 2024 14:36
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.

Looks good. I just left a few suggestions and notes about typos.

include/netplan.h Outdated Show resolved Hide resolved
include/netplan.h Outdated Show resolved Hide resolved
include/types.h Outdated Show resolved Hide resolved
include/types.h Outdated Show resolved Hide resolved
include/util.h Outdated Show resolved Hide resolved
It's not properly implemented and unused.
Moving all the functions related to NetplanNetDefinition into netdef.h
- Mostly "netplan_netdef_*" functions
- Functions which take NetplanNetDefinition* as their first argument

Moving all the functions related to NetplanState into state.h
- Mostly "netplan_state_*" functions
- Functions which take NetplanState* as their first argument
- Iterator functions that work on NetplanState*

Leaving include/netplan.h mostly empty, as a bare aggregation of the other
headers, so we don't break NetworkManager, which includes <netplan/netplan.h>.
@slyon
Copy link
Collaborator Author

slyon commented Jan 29, 2024

Thanks, those were some nice finds! I fixed it all as suggested and rebased. Let's get this merged, once CI is green.

@rkratky Feel free to still leave your comments in here. If anything, I can fix it in a follow-up PR.

@slyon slyon force-pushed the api-docs branch 4 times, most recently from 8354b0a to 1fb01cd Compare January 29, 2024 11:18
@slyon
Copy link
Collaborator Author

slyon commented Jan 29, 2024

The NetworkManager CI is failing again... But it is unrelated to these changes and needs fixing independently in the CI images.

@slyon slyon merged commit 24deed5 into canonical:main Jan 29, 2024
36 of 42 checks passed
@slyon slyon added the documentation Documentation improvements. label Jan 31, 2024
@slyon slyon mentioned this pull request Feb 7, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Documentation improvements.
Projects
None yet
2 participants