-
Notifications
You must be signed in to change notification settings - Fork 182
#86 Repository location stripping #88
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
#86 Repository location stripping #88
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
@@ -105,10 +105,10 @@ def save_into_archive(xml_tree, filename, new_filename=None): | |||
|
|||
def _save_file(container_file, xml_tree, new_filename=None): | |||
|
|||
if container_file is None: | |||
container_file = new_filename | |||
if new_filename is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, thanks!
@@ -213,6 +213,10 @@ def version(self): | |||
def connections(self): | |||
return self._connections | |||
|
|||
def clear_repository_location(self): | |||
tag = self._datasourceXML.find('./repository-location') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intentional that it's ./
here but .//
in the test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor thing after taking a second look
@@ -213,6 +213,10 @@ def version(self): | |||
def connections(self): | |||
return self._connections | |||
|
|||
def clear_repository_location(self): | |||
tag = self._datasourceXML.find('./repository-location') | |||
self._datasourceXML.remove(tag) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will fail and throw a TypeError if the find returns None
-- eg if you call this method twice or it never had a repository-location
tag.
Do we just want this to return silently instead of blowing up?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh dang that was supposed to have an if tag: on it
🚀 🌕 ⏯️ 🌠 |
* Add the ability to clear repository location on the datasource for retargetting * fixing a pep8 issue that was missed * Adding None check for repository-location in case it doesn't exist
Addresses #86