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

Update GitHub API comment #1621

Merged
merged 2 commits into from Apr 16, 2018

Conversation

@katrinleinweber
Copy link
Contributor

commented Apr 13, 2018

stats is nowhere to be found on https://api.github.com/repos/zotero/zotero, nor any users' forks like https://api.github.com/repos/zuphilip/zotero. I think GH updated the API URL as in the diff. If that's the one meant by the comment, please consider merging this.

PS: If the above is true, I'm guessing that "organization in GitHub cannot itself provide contributions" from #1296 is now outdated, and the translator could be updated in that way. If yes, shall I open an issue, or is that change trivial and can be appended here?

katrinleinweber and others added some commits Apr 13, 2018

@zuphilip zuphilip force-pushed the katrinleinweber:patch-3 branch from 196ea02 to beea162 Apr 14, 2018

@zuphilip

This comment has been minimized.

Copy link
Collaborator

commented Apr 14, 2018

I updated the translator a little bit more and updated also the test cases. Please have a look.

I don't know whether the Github API changed but these comments seem for me of little value anyways. I tried to replace them with something more meaningful.

Why do you think we should change how we save the owner information? E.g. zotero is the company but Dan would be a programmer seems still fine for me.

@katrinleinweber

This comment has been minimized.

Copy link
Contributor Author

commented Apr 14, 2018

Thanks for the update :-)

To elaborate on my "PS": Maybe I'm seeing a bug. Does importing an organisational repo result in any programmers for you? Neither HEAD nor beea162 does in my case (macOS 10.13.3, Zotero 5.0.35).

Dan for example, only appears as programmer for me, when I trigger the translation on his repos. The field remains empty, whenever I translate the upstream repo, even though he obviously is a (or the main) contributor to them.

screen shot 2018-04-14 at 17 27 46

From #1296 I understood that owner == org back then prevented any programmer name from getting extracted. But from your comment above I gather that both company and programmer are expected to get extracted, correct? I agree that would be fine, but I never observed that expected behavior so far.

@zuphilip

This comment has been minimized.

Copy link
Collaborator

commented Apr 14, 2018

Does importing an organisational repo result in any ´programmers´ for you?

No, the organization will be saved in the company field.

We also tried to look at the contributors, which need another API call, e.g. https://api.github.com/repos/zotero/zotero/contributors, but this list was too much to be meaningful in general. For doing better we might need more information to rely on, e.g. a CODEMETA file.

@katrinleinweber

This comment has been minimized.

Copy link
Contributor Author

commented Apr 15, 2018

Hm. I'd argue that at least adding the 0-th contributor (with most contributions) as programmer would be more "meaningful" than none, or a fork's owner.

@zuphilip zuphilip merged commit e6d0a61 into zotero:master Apr 16, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@zuphilip

This comment has been minimized.

Copy link
Collaborator

commented Apr 16, 2018

@katrinleinweber Thank you!

I try to look at the #1587 soon and then we possibly can revisit GitHub translator afterwards again.

@katrinleinweber

This comment has been minimized.

Copy link
Contributor Author

commented Apr 17, 2018

Thank you :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.