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

This addresses a problem caused by files in which a source element ha… #80

Merged
merged 2 commits into from
Mar 28, 2017

Conversation

LettError
Copy link
Owner

…s no name attribute. Mutator uses the value of the name attribute to store some data and retrieve it later. This adds a temporary simple unique name.

…s no name attribute. Mutator uses the value of the name attribute to store some data and retrieve it later. This adds a temporary simple unique name.
@LettError LettError requested a review from anthrotype March 28, 2017 21:13
@coveralls
Copy link

coveralls commented Mar 28, 2017

Coverage Status

Coverage decreased (-0.09%) to 88.603% when pulling e324222 on fix-empty-source-name into 73b7fc2 on master.

@@ -502,12 +502,19 @@ def readSources(self):
</source>

"""
sourceCount = 0
for sourceElement in self.root.findall(".sources/source"):
Copy link
Collaborator

Choose a reason for hiding this comment

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

you could also do this instead:

-sourceCount = 0
-for sourceElement in self.root.findall(".sources/source"):
-    sourceCount+=1
+for sourceCount, sourceElement in enumerate(self.root.findall(".sources/source")):

# (some authoring tools do not need them)
# then we should make a temporary one
sourceName = "temp_master.%d"%(sourceCount)
print("created temp source name", sourceName)
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not use self.logger instead of print here?

Copy link
Collaborator

@anthrotype anthrotype left a comment

Choose a reason for hiding this comment

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

other than the minor comments above, LGTM

@LettError
Copy link
Owner Author

Thank you!

@coveralls
Copy link

coveralls commented Mar 28, 2017

Coverage Status

Coverage decreased (-0.05%) to 88.647% when pulling 1a6e6d3 on fix-empty-source-name into 73b7fc2 on master.

@LettError LettError merged commit 656b845 into master Mar 28, 2017
@LettError LettError deleted the fix-empty-source-name branch March 28, 2017 22:11
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.

3 participants