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

Improving "disaster_recovery" & "ovirt_disk" (documentation & phrasings) #562

Merged
merged 4 commits into from
Nov 23, 2022
Merged

Improving "disaster_recovery" & "ovirt_disk" (documentation & phrasings) #562

merged 4 commits into from
Nov 23, 2022

Conversation

barpavel
Copy link
Member

@barpavel barpavel commented Jul 17, 2022 •
edited
Loading

A few small improvements in disaster_recovery & ovirt_disk:

  1. Documentation.
  2. Code comments & user messages.
  3. Fixing warnings, removing unused imports and local variables.

@barpavel barpavel requested a review from mnecas as a code owner July 17, 2022 10:56
@barpavel barpavel marked this pull request as draft July 17, 2022 10:57
@barpavel barpavel force-pushed the small_ovirt_disk_doc_improvements branch 2 times, most recently from d899c44 to 6ccbd27 Compare July 17, 2022 13:41
@mwperina
Copy link
Member

mwperina commented Aug 3, 2022

@barpavel Anything else missing? If not, then please move the PR to review ...

@barpavel
Copy link
Member Author

barpavel commented Aug 3, 2022

@barpavel Anything else missing? If not, then please move the PR to review ...

Yes, there were few more things I wanted to add, the PR is not finished.
I will try to finish it this week.

@barpavel barpavel force-pushed the small_ovirt_disk_doc_improvements branch 5 times, most recently from 6136443 to ff379d0 Compare November 15, 2022 08:56
@barpavel barpavel marked this pull request as ready for review November 15, 2022 08:57
@barpavel
Copy link
Member Author

barpavel commented Nov 15, 2022 •
edited
Loading

Actually one of the reasons this PR started, was that I saw the following strange documentation, see https://docs.ansible.com/ansible/latest/collections/ovirt/ovirt/ovirt_disk_module.html

bootable boolean

True if the disk should be bootable. By default when disk is created it isn’t bootable.
Choices:
no
yes

I mean pay attention to the text that requires True vs. Choices that have other enum values: yes /no....
I wanted to fix that but I'm not sure should I and how...

@barpavel
Copy link
Member Author

Actually one of the reasons this PR started, was that I saw the following strange documentation, see https://docs.ansible.com/ansible/latest/collections/ovirt/ovirt/ovirt_disk_module.html

bootable boolean

True if the disk should be bootable. By default when disk is created it isn’t bootable.
Choices:
no
yes

I mean pay attention to the text that requires True vs. Choices that have other enum values: yes /no.... I wanted to fix that but I'm not sure should I and how...

@mnecas @mwperina What do you think?

mwperina
mwperina approved these changes Nov 15, 2022
View reviewed changes
Copy link
Member

@mwperina mwperina left a comment

Choose a reason for hiding this comment

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

+1

@mwperina mwperina force-pushed the small_ovirt_disk_doc_improvements branch from ff379d0 to 1476294 Compare November 15, 2022 15:34
mwperina
mwperina requested changes Nov 15, 2022
View reviewed changes
plugins/modules/ovirt_disk.py Outdated Show resolved Hide resolved
plugins/modules/ovirt_disk.py Outdated Show resolved Hide resolved
@barpavel barpavel force-pushed the small_ovirt_disk_doc_improvements branch 3 times, most recently from 5d5ed77 to 64319a9 Compare November 22, 2022 16:12
@mwperina mwperina force-pushed the small_ovirt_disk_doc_improvements branch from 64319a9 to 0e937fd Compare November 23, 2022 08:49
@barpavel @mwperina
Improve "disaster_recovery" documentation "README.md" file.
6b8636b 
Improving "disaster_recovery" role variables documentation table:
1. Minor (IDE-automatic) table reformatting to eliminate
a few IDE warnings.
2. Adding backquotes around values to denote them "as code".

Signed-off-by: Pavel Bar <pbar@redhat.com>
@barpavel @mwperina
Improving "ovirt_disk" phrasings
b3baa25 
Improving "ovirt_disk" - documentation, code comments & messages:
1. Minor rephrasing.
2. Fixing typos.
3. Consistent upper-case usage.

Signed-off-by: Pavel Bar <pbar@redhat.com>
@barpavel @mwperina
Small "ovirt_disk" fixes.
0965da2 
1. Improve "ovirt_disk::id" description to specify that "alias"
is also an option. This also makes the description to be consistent
with the "ovirt_disk::name" description.
2. Fixing warnings:
  a. Redundant "ssl" import causing "Unused import statement 'import ssl'".
  b. Several "Local variable 'XYZ' value is not used".
  c. Several "Shadows name 'disk' from outer scope".

Signed-off-by: Pavel Bar <pbar@redhat.com>
@barpavel @mwperina
Adding changelog.
9e59aad 
Signed-off-by: Pavel Bar <pbar@redhat.com>
@mwperina mwperina force-pushed the small_ovirt_disk_doc_improvements branch from 0e937fd to 9e59aad Compare November 23, 2022 09:38
mwperina
mwperina approved these changes Nov 23, 2022
View reviewed changes
Copy link
Member

@mwperina mwperina left a comment

Choose a reason for hiding this comment

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

+1

@mwperina mwperina merged commit ea2f6b5 into oVirt:master Nov 23, 2022
@barpavel barpavel changed the title Improving "ovirt_disk" (mostly documentation) Improving "disaster_recovery" & "ovirt_disk" (documentation & phrasings) Nov 23, 2022
@barpavel barpavel deleted the small_ovirt_disk_doc_improvements branch November 23, 2022 16:25
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mwperina mwperina mwperina approved these changes

@mnecas mnecas Awaiting requested review from mnecas mnecas is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@barpavel @mwperina @mnecas