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

feat: Add and Remove kernels, add with --copy-defaults #75

Merged
merged 10 commits into from
Jan 9, 2024

Conversation

spetrosi
Copy link
Contributor

@spetrosi spetrosi commented Dec 21, 2023

Enhancement:

  1. Add an ability to add and remove kernels. To do that, set state to either present or absent.
  2. When adding kernels, you can set copy_defaults: true to copy arguments from the default kernel.
  3. Remove the ability to provide lists with kernel keys, now each kernel key can contain a single dict. This makes it consistent with other roles. You can use kernel: ALL to modify all kernels at once.

@spetrosi spetrosi requested a review from richm as a code owner December 21, 2023 19:14
@spetrosi
Copy link
Contributor Author

[citest]

Copy link

codecov bot commented Dec 21, 2023

Codecov Report

Attention: 33 lines in your changes are missing coverage. Please review.

Comparison is base (433771f) 70.40% compared to head (94b5421) 78.43%.

Files Patch % Lines
library/bootloader_settings.py 80.47% 33 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #75      +/-   ##
==========================================
+ Coverage   70.40%   78.43%   +8.03%     
==========================================
  Files           2        2              
  Lines         125      255     +130     
==========================================
+ Hits           88      200     +112     
- Misses         37       55      +18     
Flag Coverage Δ
sanity ∅ <ø> (∅)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

library/bootloader_settings.py Fixed Show fixed Hide fixed
library/bootloader_settings.py Fixed Show fixed Hide fixed
library/bootloader_settings.py Fixed Show fixed Hide fixed
library/bootloader_settings.py Fixed Show fixed Hide fixed
library/bootloader_settings.py Fixed Show fixed Hide fixed
library/bootloader_settings.py Fixed Show fixed Hide fixed
library/bootloader_settings.py Fixed Show fixed Hide fixed
library/bootloader_settings.py Fixed Show fixed Hide fixed
library/bootloader_settings.py Fixed Show fixed Hide fixed
library/bootloader_settings.py Fixed Show fixed Hide fixed
@spetrosi spetrosi marked this pull request as draft December 21, 2023 20:47
library/bootloader_settings.py Fixed Show fixed Hide fixed
library/bootloader_settings.py Fixed Show fixed Hide fixed
@spetrosi spetrosi force-pushed the add-remove-kernels branch 2 times, most recently from 149e52e to b29de0b Compare December 22, 2023 13:27
@spetrosi
Copy link
Contributor Author

[citest]

@spetrosi spetrosi force-pushed the add-remove-kernels branch 2 times, most recently from 3cae929 to 291587e Compare December 22, 2023 16:26
@spetrosi
Copy link
Contributor Author

[citest]

1 similar comment
@spetrosi
Copy link
Contributor Author

spetrosi commented Jan 2, 2024

[citest]

@spetrosi spetrosi force-pushed the add-remove-kernels branch from 2366e72 to 553cd68 Compare January 3, 2024 15:51
@spetrosi spetrosi closed this Jan 3, 2024
@spetrosi spetrosi reopened this Jan 3, 2024
@spetrosi
Copy link
Contributor Author

spetrosi commented Jan 3, 2024

[citest]

@spetrosi spetrosi force-pushed the add-remove-kernels branch from 553cd68 to 831efff Compare January 3, 2024 18:50
@spetrosi
Copy link
Contributor Author

spetrosi commented Jan 3, 2024

[citest]

1 similar comment
@spetrosi
Copy link
Contributor Author

spetrosi commented Jan 3, 2024

[citest]

@spetrosi spetrosi force-pushed the add-remove-kernels branch from 831efff to afac1ce Compare January 4, 2024 08:46
@spetrosi
Copy link
Contributor Author

spetrosi commented Jan 4, 2024

[citest]

1 similar comment
@spetrosi
Copy link
Contributor Author

spetrosi commented Jan 4, 2024

[citest]

@spetrosi spetrosi marked this pull request as ready for review January 4, 2024 14:27
@spetrosi
Copy link
Contributor Author

spetrosi commented Jan 5, 2024

@richm it's now ready for your review, please take a look.
I do not know how to use mock in unit tests to test Ansible's module method. That's why bootloader_settings.py uses a weird format of running commands, it calls a function to return a cmd to run, or an error to return, and then does the action in the main run_module func. I will try to refactor this, but the role's functionality will stay the same.

tests/tests_add_rm.yml Outdated Show resolved Hide resolved
tests/tests_add_rm.yml Outdated Show resolved Hide resolved
tests/tests_settings.yml Outdated Show resolved Hide resolved
@richm
Copy link
Contributor

richm commented Jan 5, 2024

There are a lot of codeql Variable defined multiple times warnings - would be nice to fix or suppress them.

@spetrosi
Copy link
Contributor Author

spetrosi commented Jan 8, 2024

[citest]

Copy link
Contributor

@richm richm left a comment

Choose a reason for hiding this comment

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

ready to merge?

@spetrosi spetrosi force-pushed the add-remove-kernels branch from 66bb91b to 449c84d Compare January 9, 2024 08:07
@spetrosi
Copy link
Contributor Author

spetrosi commented Jan 9, 2024

[citest]

@spetrosi spetrosi force-pushed the add-remove-kernels branch from 449c84d to 94b5421 Compare January 9, 2024 09:31
@spetrosi
Copy link
Contributor Author

spetrosi commented Jan 9, 2024

[citest]

@spetrosi spetrosi merged commit 97ef015 into linux-system-roles:main Jan 9, 2024
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants