Skip to content

Conversation

@yash-nisar
Copy link
Contributor

Add 'annotator','annotation_date', and 'comment'
to the 'annotation' class.

Signed-off-by: Yash Nisar [email protected]

@yash-nisar yash-nisar force-pushed the annotation-spec-2.1 branch 2 times, most recently from ccca9a3 to 8cb3582 Compare June 12, 2018 16:21
@yash-nisar yash-nisar changed the title Add 'annotation' class as a part of the SPDX 2.1 specification. Add 'Annotation' class as a part of the SPDX 2.1 specification. Jul 3, 2018
@pombredanne
Copy link
Member

This needs to be updated to resolve merge conflicts.

@yash-nisar yash-nisar force-pushed the annotation-spec-2.1 branch from c049034 to 960367c Compare August 10, 2018 09:42
Add 'annotator','annotation_date', and 'comment'
to the 'annotation' class.

Signed-off-by: Yash Nisar <[email protected]>
@yash-nisar yash-nisar force-pushed the annotation-spec-2.1 branch from 960367c to 413edaa Compare August 10, 2018 09:44
Copy link
Member

@pombredanne pombredanne left a comment

Choose a reason for hiding this comment

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

Looking good!... I made some veryminor comments for your consideration

def test_correct_annotation_spdx_id(self):
spdx_id = 'SPDXRef-45'
self.add_annotator()
self.builder.set_annotation_spdx_id(self.document, spdx_id)
Copy link
Member

Choose a reason for hiding this comment

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

What do you assert as test result here?

self.write_tagvalue_file(doc, filename)
###############################################
doc, error = self.parse_tagvalue_file(filename)
# print(doc.annotations[-1].annotation_type)
Copy link
Member

Choose a reason for hiding this comment

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

you likely want to delete this

for lics in doc.extracted_licenses:
print '\tIdentifier: {0}'.format(lics.identifier)
print '\tName: {0}'.format(lics.full_name)
print 'Annotations:'
Copy link
Member

Choose a reason for hiding this comment

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

for the examples to also work on Python 3, please import the future print function and use it.

@pombredanne
Copy link
Member

Actually these are so inor that you can address these comments seprately.
I am merging this.

@pombredanne pombredanne merged commit 589faa8 into spdx:master Aug 10, 2018
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