Skip to content
This repository has been archived by the owner on Jan 24, 2024. It is now read-only.

Merge skeleton-python-library #13

Merged
merged 8 commits into from
Feb 28, 2020

Conversation

mcdonnnj
Copy link
Member

🗣 Description

This PR brings this project into the fold by merging cisagov/skeleton-python-library. This includes migrating the project from Travis CI to GitHub Actions,

💭 Motivation and Context

Since I will need to add some functionality to this project I thought it would be a good idea to bring it in line with other Python projects by merging in skeleton-python-library.

🧪 Testing

All automated tests pass with no issues. I tested the change to defusedxml manually and ensured that a JSON would still be generated correctly from sample XML.

🚥 Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (causes existing functionality to change)

✅ Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

…hing to the defusedxml package.

Added the #nosec tag to the requests.get() call because we need to use verify=false to successfully connect.
Rename the assessment_data_export() function to export_assessment_data() so I can access the main() function in tests.
Correctly tie the version reported by Docopt to match the __version__ set in _versiono.py
@mcdonnnj mcdonnnj self-assigned this Feb 27, 2020
@mcdonnnj mcdonnnj requested review from dav3r, felddy, jsf9k and a team February 27, 2020 20:05
@lgtm-com
Copy link

lgtm-com bot commented Feb 27, 2020

This pull request introduces 1 alert and fixes 1 when merging f632cb0 into e8c1ace - view on LGTM.com

new alerts:

  • 1 for Module imports itself

fixed alerts:

  • 1 for Module imports itself

Copy link
Member

@jsf9k jsf9k left a comment

Choose a reason for hiding this comment

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

Approved, but see my one comment.

@@ -94,7 +96,7 @@ def export_jira_data(jira_base_url, jira_credentials_file, jira_filter, xml_file
# Export XML data from Jira
try:
response = requests.get(
jira_url, auth=(jira_username, jira_password), verify=False
jira_url, auth=(jira_username, jira_password), verify=False # nosec
Copy link
Member

Choose a reason for hiding this comment

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

Why is nosec needed here? It might be good to include a comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, I mentioned it in the commit but forgot to add a code comment. Hopefully the comment in 2f19cc6 satisfies.

@lgtm-com
Copy link

lgtm-com bot commented Feb 27, 2020

This pull request introduces 1 alert and fixes 1 when merging 2f19cc6 into e8c1ace - view on LGTM.com

new alerts:

  • 1 for Module imports itself

fixed alerts:

  • 1 for Module imports itself

Copy link
Member

@dav3r dav3r left a comment

Choose a reason for hiding this comment

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

Excelsior! Fine work @mcdonnnj. 💪

@mcdonnnj mcdonnnj merged commit b5ad023 into develop Feb 28, 2020
@mcdonnnj mcdonnnj deleted the improvements/merge_skeleton-python-library branch February 28, 2020 05:10
mcdonnnj pushed a commit that referenced this pull request Sep 28, 2020
mcdonnnj pushed a commit that referenced this pull request Sep 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants