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

Clarify subiquity docs #5957

Merged
merged 3 commits into from
Feb 6, 2025
Merged

Clarify subiquity docs #5957

merged 3 commits into from
Feb 6, 2025

Conversation

holmanb
Copy link
Member

@holmanb holmanb commented Jan 9, 2025

Merge type

  • Squash merge using "Proposed Commit Message"
  • Rebase and merge unique commits. Requires commit messages per-commit each referencing the pull request number (#<PR_NUM>)

@github-actions github-actions bot added the documentation This Pull Request changes documentation label Jan 9, 2025
holmanb
holmanb commented Jan 9, 2025
View reviewed changes
tests/unittests/config/test_cc_ubuntu_autoinstall.py
@@ -73,14 +46,6 @@ class TestHandleAutoinstall:
@pytest.mark.parametrize(
"cfg,snap_list,subp_calls,logs,snap_wait_called",
[
pytest.param(
Copy link
Member Author

@holmanb holmanb Jan 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This never happens in the codebase due to activate_by_schema_keys: autoinstall.

@holmanb holmanb force-pushed the holmanb/subiquity-docs branch from 0f463e9 to 578b76b Compare January 9, 2025 20:56
holmanb
holmanb commented Jan 9, 2025
View reviewed changes
tests/unittests/config/test_cc_ubuntu_autoinstall.py
@@ -38,33 +38,6 @@
)


class TestvalidateConfigSchema:
Copy link
Member Author

Choose a reason for hiding this comment

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

Coverage on this module remains at 100% after removing these tests.

s-makin
s-makin approved these changes Jan 15, 2025
View reviewed changes
Copy link
Contributor

@s-makin s-makin left a comment

Choose a reason for hiding this comment

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

Just one small nit, leaving a pre-emptive approval

@github-actions GitHub Actions
Copy link

Hello! Thank you for this proposed change to cloud-init. This pull request is now marked as stale as it has not seen any activity in 14 days. If no activity occurs within the next 7 days, this pull request will automatically close.

If you are waiting for code review and you are seeing this message, apologies! Please reply, tagging TheRealFalcon, and he will ensure that someone takes a look soon.

(If the pull request is closed and you would like to continue working on it, please do tag TheRealFalcon to reopen it.)

@github-actions github-actions bot added the stale-pr Pull request is stale; will be auto-closed soon label Jan 30, 2025
@holmanb holmanb added Needs Review and removed stale-pr Pull request is stale; will be auto-closed soon labels Feb 5, 2025
@holmanb holmanb force-pushed the holmanb/subiquity-docs branch from b451e9c to fac150b Compare February 5, 2025 21:51
TheRealFalcon
TheRealFalcon approved these changes Feb 6, 2025
View reviewed changes
Copy link
Member

@TheRealFalcon TheRealFalcon left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM! I saw that there was already a reviewer face on this, so I never bothered looking until now. In the future, feel free to add me to the assignees if you need an extra reviewer.

@holmanb
doc: clarify subiquity docs
2be0c50 
Cloud-init ignores this key and its contents. Dispell confusion with clearer
messaging and more user-friendly language.
@holmanb
chore: remove redundant manual schema validation
f814843 
Schema validation is automatically checked and reported through
cloud-init status. Eliminate unnecessary manual verification as well as
the autoinstall key check which is unnecessary due to activation_key.
@holmanb
test: eliminate obsolete cases, add non-error case
deb2ed9
@holmanb holmanb force-pushed the holmanb/subiquity-docs branch from fac150b to deb2ed9 Compare February 6, 2025 20:11
@holmanb
Copy link
Member Author

holmanb commented Feb 6, 2025

The test is an issue with CI that is unrelated to this PR. Merging.

@holmanb holmanb merged commit 6677ce2 into canonical:main Feb 6, 2025
21 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TheRealFalcon TheRealFalcon TheRealFalcon approved these changes

@s-makin s-makin s-makin approved these changes

Assignees

@s-makin s-makin

Labels
documentation This Pull Request changes documentation Needs Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@holmanb @TheRealFalcon @s-makin