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

T6910: fix documentation and argspecs are out of alignment #358

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

gaige
Copy link
Contributor

@gaige gaige commented Nov 15, 2024

Change Summary

Minor modifications by bringing the resource_module_builder-based modules back into line with what is generated, fixing some variances in documentation and inconsistencies in some of the modules and the documentation and
test sources.

Further, verified and regenerated argspecs based for the newer cli modules.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Other (please describe):

Related Task(s)

T6910: documentation and argspecs are out of alignment

Related PR(s)

Proposed changes

Updates to the resource module repo (separate) to bring them into line with the current code.
Added a custom version of the resource_builder in order to ensure we don't remove any
updates for our modules.
Regenerated until we matched all salient updates.

How to test

ansible-test units --docker
ansible-test sanity
ansible-test network-integration --inventory `pwd`/inventory.network

Unit tests and sanity tests pass; integration tests are failing in the same ways that the current mainline branch are
failing against 1.4 and 1.5 (but work against 1.3).

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

Copy link
Contributor

@omnom62 omnom62 left a comment

Choose a reason for hiding this comment

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

Looks more cosmetic to me but still good to push

@gaige gaige force-pushed the update-resources branch 2 times, most recently from d39a618 to 126a5e2 Compare November 22, 2024 08:59
fix: update firewall_global from resource model

chore: update to sync with resource module

chore: update README

fix: comment and formatting

fix: formatting issues

fix: missing imports

fix: import and metadata violations

fix: interface fixes

fix: move the arg spec comment

fix: update interface docs

fix: remove comments for non-RM versions
@gaige gaige force-pushed the update-resources branch 7 times, most recently from 123108c to 495aa76 Compare November 23, 2024 14:29
@gaige gaige force-pushed the update-resources branch 5 times, most recently from 5e3563a to d43ad5f Compare November 23, 2024 21:27
@gaige gaige marked this pull request as ready for review November 23, 2024 21:32
@gaige gaige changed the title Update with Network Resources templates T6910: fix documentation and argspecs are out of alignment Nov 24, 2024
@gaige gaige requested a review from a team November 24, 2024 10:02
Copy link
Contributor

@omnom62 omnom62 left a comment

Choose a reason for hiding this comment

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

Thanks - I am not intimately familiar with the code but LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants