Skip to content

Some refactoring #338

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
Merged
merged 4 commits into from
Apr 13, 2023
Merged

Some refactoring #338

merged 4 commits into from
Apr 13, 2023

Conversation

Image for: Conversation
Copy link
Collaborator

slyon commented Apr 3, 2023 •
edited
Loading

Description

Image for: Description

Cleanup a few basic things like:

  • using shutil.rmtree() over rm -rf subprocess call
  • clean up all the coverage data from the Makefile (make clean)
  • Use the more readable text=True alias to subprocess calls (over universal_newlines=True) – it's available since Python 3.7
  • Improve parameter passing to networkctl, accounting for network interfaces names that contain a leading -

Checklist

Image for: 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 mentioned this pull request Apr 3, 2023
Merged
5 tasks
slyon changed the title WIP: Refactorings Apr 13, 2023
slyon marked this pull request as ready for review April 13, 2023 08:35
slyon requested review from daniloegea and ogayot April 13, 2023 08:35
ogayot approved these changes Apr 13, 2023
View reviewed changes
Copy link
Member

ogayot left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!
daniloegea approved these changes Apr 13, 2023
View reviewed changes
Copy link
Contributor

daniloegea left a comment

Choose a reason for hiding this comment

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

lgtm
slyon merged commit 87aea1d into canonical:main Apr 13, 2023
slyon mentioned this pull request Apr 24, 2023
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants