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

Added has_license and has_copyright to delta object #114

Closed
wants to merge 1 commit into from

Conversation

shubhscoder
Copy link

@shubhscoder shubhscoder commented Mar 16, 2019

Fixes #109
Signed-off-by: Shubham shubhamsangamnerkar9@gmail.com

@shubhscoder
Copy link
Author

@MaJuRG , this is my first Pull request, please suggest me any required changes. Thank you !

Copy link
Contributor

@steven-esser steven-esser left a comment

Choose a reason for hiding this comment

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

@shubhscoder This is the correct first start. However, we want to populate these new field values properly in the right places.

See the various license and copyright functions in https://github.com/nexB/deltacode/blob/develop/src/deltacode/utils.py

Where we run delta.update(blah blah) we will want update these new flags for our Delta object and remove the "factors" that are added at each of these conditions.

@shubhscoder shubhscoder force-pushed the add_has_license branch 3 times, most recently from aec399a to acf9fd6 Compare March 22, 2019 08:23
@shubhscoder
Copy link
Author

@MaJuRG , I have tried updating has_license and has copyright. The remaining delta.update would I guess go with addition of status right ? Also I know why the tests are failing because we have removed the scoring system. Should I try updating and addition of "status" in this PR itself? Then probably I can try updating the tells together. Also I am not sure why DCO is failing. The expected and received signs are exactly same.
Thank you so much !

@pombredanne
Copy link
Member

Nit: please use an imperative style and not the past tense for your commit title. Also suffix it with the ticket number. And then explain in the body what this commit does
e.g. not Added has_license and has_copyright to delta object
but rather Add has_license/has_copyright to delta object #109

See https://github.com/nexB/aboutcode/wiki/Writing-good-commit-messages

@pombredanne
Copy link
Member

also your git user config should match your signoff for the DCO bot to be happy ;)

Update has_license and has_copyright appropriately

Signed-off-by: Shubham  <shubhamsangamnerkar9@gmail.com>
@shubhscoder
Copy link
Author

Nit: please use an imperative style and not the past tense for your commit title. Also suffix it with the ticket number. And then explain in the body what this commit does
e.g. not Added has_license and has_copyright to delta object
but rather Add has_license/has_copyright to delta object #109

See https://github.com/nexB/aboutcode/wiki/Writing-good-commit-messages

Thank you so much! I updated the commit message.

@shubhscoder
Copy link
Author

shubhscoder commented Mar 22, 2019

also your git user config should match your signoff for the DCO bot to be happy ;)

@pombredanne , the "expected" and the "got" fields are exactly same. Also I checked with my local git, the global configuration is same as the signoff.
fix

Thank you!

@pombredanne
Copy link
Member

@shubhscoder the DCO bot may have a bug ... but in any case you need to sign with your full name. Thank you for fixing this

@shubhscoder
Copy link
Author

shubhscoder commented Mar 22, 2019 via email

@steven-esser
Copy link
Contributor

Closing due to stagnation.

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.

Add has_license and has_copyright flags to Delta object
3 participants