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

Updates for ansible-akamai #6

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

aaronk1
Copy link

@aaronk1 aaronk1 commented Jan 26, 2023

Update documentation, fix pylint and pycodestyle issues, remove python2 support, add python3 support

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@@ -1,29 +1,130 @@
#!/usr/bin/env python
#!/usr/bin/python
Copy link

Choose a reason for hiding this comment

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

I believe keeping /usr/bin/env python will perform correctly but has an added advantage in some environments.

https://www.baeldung.com/linux/bash-shebang-lines.

Is there a specific reason to change it?

Copy link
Author

Choose a reason for hiding this comment

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

Not a major preference, just following the ansible guidelines.

Copy link
Author

Choose a reason for hiding this comment

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

Oddly enough, the ansible-core native modules don't include shebang at all. Example

library/akamai.py Outdated Show resolved Hide resolved
Copy link

@swtch1 swtch1 left a comment

Choose a reason for hiding this comment

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

Looks good, even though my approval may not matter.

Remove whitespace before curly brackets and lower case edgegrid-python
Remove comment
* Update method arg to include the choices GET, POST, PUT
* Update body arg to be type json instead of type str
@aaronk1
Copy link
Author

aaronk1 commented Jan 27, 2023

@jacob-hudson please let me know what you think and if you'd like to see any changes.

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