-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add contribution_utils to license_utils for more complete unit testing #271
Add contribution_utils to license_utils for more complete unit testing #271
Conversation
Pull Request Test Coverage Report for Build 5694652740
💛 - Coveralls |
"Koray K\u0131rl\u0131": { | ||
"emails": [ | ||
"KorayKirli@users.noreply.github.com", | ||
"koray_kirli@hms.harvard.edu" | ||
], | ||
"names": [ | ||
"Koray K\u0131rl\u0131" | ||
] |
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.
Looks like something that may need correcting in encoding?
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.
No, I think it's safer this way. It will output correctly. It can be read by the default python reader and it will display as real unicode if you print it out in a regular file or on the terminal.
"name": actor.name, | ||
"email": actor.email, | ||
} | ||
|
||
@classmethod | ||
def json_for_commit(cls, commit: git.Commit) -> Dict: | ||
return { | ||
'commit': commit.hexsha, | ||
'date': commit.committed_datetime.isoformat(), | ||
'author': cls.json_for_actor(commit.author), |
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.
Mixed use of single/double quotes here?
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.
Yeah, I can look at that. It's a losing battle, though. So many complex constraints.
# excluded_fork_contributors_by_name = (self.excluded_contributions.contributors_by_name | ||
# if self.excluded_contributions | ||
# else {}) |
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.
Commented out intentionally?
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.
It was something I was experimenting with. I only need email but I should take in the _by_name data and convert it to _by_email because the _by_name data is more compact and more intelligible. I'll work on that later.
n = 0 | ||
for commit in reversed(list(git_repo.iter_commits())): | ||
n += 1 |
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.
Does this have real use? Maybe rename if so... It looks unused granted I don't have my syntax highlighting
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.
the counter? i was using it for some debugging. it's not used in the core part, yeah.
This is a layered PR adding changes to allow checking of contributors.
qa_checkers.py
addingContributionsChecker.validate()
, which will check that aCONTRIBUTORS.json
file exists and summarizes who's contributed to the repo. This will allow us to make sure contributions come from employees, or to check that other contributions are made by people agreeing to our contribution policy.