Skip to content

Fix/remove pack dunder#58622

Merged
dwoz merged 5 commits intosaltstack:masterfrom
major0:fix/remove_pack_dunder
Oct 7, 2020
Merged

Fix/remove pack dunder#58622
dwoz merged 5 commits intosaltstack:masterfrom
major0:fix/remove_pack_dunder

Conversation

@major0
Copy link
Contributor

@major0 major0 commented Oct 3, 2020

What does this PR do?

Basic code maintenance:

  1. Remove calls salt.utils.compat.pack_dunder() from all boto/boto3 modules since these calls have not been necessary since 2015.8.0.
  2. Remove calls to salt.utils.compat.pack_dunder() from all code outside of boto/boto3 modules. This includes some tests and the module and state example templates for writing new code.
  3. Update all touched code in order to make it pass the pre-commit tests.

This does not fix any bugs, or add any features.

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

No

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

@major0 major0 requested a review from a team as a code owner October 3, 2020 19:31
@major0 major0 requested review from krionbsd and removed request for a team October 3, 2020 19:31
@ghost ghost requested a review from waynew October 3, 2020 19:31
krionbsd
krionbsd previously approved these changes Oct 3, 2020
Copy link
Contributor

@krionbsd krionbsd left a comment

Choose a reason for hiding this comment

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

LGTM

s0undt3ch
s0undt3ch previously approved these changes Oct 4, 2020
Copy link
Contributor

@s0undt3ch s0undt3ch left a comment

Choose a reason for hiding this comment

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

And is pack_dunder still used anywhere?

@major0
Copy link
Contributor Author

major0 commented Oct 4, 2020

There are other files outside of the boto/boto3 code tree that are still using it yes.

salt/modules/libcloud_compute.py
salt/modules/libcloud_dns.py
salt/modules/libcloud_loadbalancer.py
salt/modules/libcloud_storage.py
salt/states/libcloud_dns.py
salt/states/libcloud_loadbalancer.py
salt/states/libcloud_storage.py
templates/module/salt/modules/{{module_name}}.py
templates/state/salt/states/{{module_name}}.py
tests/unit/modules/test_libcloud_compute.py
tests/unit/modules/test_libcloud_dns.py
tests/unit/modules/test_libcloud_loadbalancer.py
tests/unit/modules/test_libcloud_storage.py

I can always add these to this PR.

@major0 major0 dismissed stale reviews from s0undt3ch and krionbsd via 41f941f October 4, 2020 12:29
@major0 major0 force-pushed the fix/remove_pack_dunder branch 3 times, most recently from 9824282 to 2013e9d Compare October 4, 2020 13:42
@s0undt3ch
Copy link
Contributor

There are other files outside of the boto/boto3 code tree that are still using it yes.

Thanks for looking.

I can always add these to this PR.

No, no need to mix.
It just means it needs to be addressed on a separate PR, and then we can remove that utility function.

@major0
Copy link
Contributor Author

major0 commented Oct 4, 2020

Too late? :)

@major0 major0 force-pushed the fix/remove_pack_dunder branch from 2013e9d to 408f2bc Compare October 4, 2020 16:22
@major0 major0 requested review from krionbsd and s0undt3ch October 4, 2020 16:31
krionbsd
krionbsd previously approved these changes Oct 4, 2020
s0undt3ch
s0undt3ch previously approved these changes Oct 4, 2020
@major0 major0 dismissed stale reviews from s0undt3ch and krionbsd via e0027d2 October 6, 2020 01:09
@major0 major0 force-pushed the fix/remove_pack_dunder branch from 408f2bc to e0027d2 Compare October 6, 2020 01:09
major0 added 4 commits October 7, 2020 08:49
Has not been needed since SaltStack 2015.8.0
Removes usage of pack_dunder from remaining code.  Usage of pack_dunder
has not been decessary since 2015.8.0
@major0 major0 force-pushed the fix/remove_pack_dunder branch from e0027d2 to cb0f544 Compare October 7, 2020 15:49
@sagetherage sagetherage added the Magnesium Mg release after Na prior to Al label Oct 7, 2020
@dwoz dwoz merged commit f997e06 into saltstack:master Oct 7, 2020
@major0 major0 deleted the fix/remove_pack_dunder branch October 7, 2020 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Magnesium Mg release after Na prior to Al

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants