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

Checking SHA uniqueness in PR commits in case the branch has been deleted #83

Open
prathamesh-sonpatki opened this issue Aug 15, 2016 · 13 comments
Labels

Comments

@prathamesh-sonpatki
Copy link
Contributor

For eg. rails/rails#26159 is not appearing even after syncing. Note that activities I performed after this PR was merged are appearing, so I am not sure if this issue is only with commits.

I had noticed this issue in July as well, lot of my commits did not appear at all.

@gautamrege
Copy link
Member

I suspect you deleted the branch before the task to collect your commits was run. The background job for data collection for commits runs once a day for each user. I think you created the branch and deleted it before the next cycle ran. Freakish but possible. Can you check Github audit logs in Settings and check let us know the timestamp you created the branch and when you deleted it.

@prathamesh-sonpatki
Copy link
Contributor Author

prathamesh-sonpatki commented Aug 16, 2016

Yes that should have happened. Because I delete branch whenever my PR gets merged, sometimes project maintainers also delete branches when they merge a PR. Its normal workflow.

I think there might be lot of commits missed for others as well due to this.

@gautamrege
Copy link
Member

Well, The project maintainers delete the branches in their repos (target). Ideally, you should wait for at least 2 days (to ensure the timezone doesn't play funny). Was just checking, audit logs are available only for organisations and not for personal accounts. Damn! So, there is no way to check exactly, when the commit was done and when the branch was deleted.

Well, I am open to suggestions about how to fix this issue.

@prathamesh-sonpatki
Copy link
Contributor Author

I haven't checked the code but we should be able to fetch pull requests by a user and Github will hopefully send their status(merged) as well. If merged, and the project satisfies the criteria, it should be counted.

@prathamesh-sonpatki
Copy link
Contributor Author

Another issue I noticed is -- if I merge some PR that commit does not appear. For eg. rails/rails#26188

I clicked sync button manually from the UI, all my new activities appeared but this commit did not appear.

@gautamrege
Copy link
Member

We look at commits only -- not PRs. This PR has someone else's commit in the PR even though you have merged it.. so how will it appear in your activity? PR merge action is not scored.

Please confirm / clarify.

FYI - I am working on a way to resolve the deleted branch issue.

@prathamesh-sonpatki
Copy link
Contributor Author

prathamesh-sonpatki commented Aug 18, 2016

I think PR merge activity should be scored, it should count more because it means the code review was also done 😄

@gautamrege
Copy link
Member

Makes sense. Should be more stringent though - only for repos with more than a 100 stars. I will open a new ticket for that.

On Aug 18, 2016, at 10:39 AM, प्रथमेश Sonpatki [email protected] wrote:

I think PR merge activity should be scored, it should count more because it means the review was also done 😄


You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

@gautamrege gautamrege added the bug label Aug 22, 2016
@SagareGanesh
Copy link
Contributor

@gautamrege I am also suffering from same issue. codetriage/codetriage#504 is not appearing even after syncing. Note that PR is not merged yet, it means branch is not deleted. All activities which I performed after this PR was raised are appearing.

@gautamrege
Copy link
Member

@SagareGanesh Is your problem resolved? If not,let me know - we shall evaluate.

Renaming this ticket to "Checking SHA uniqueness in PR commits in case the branch has been deleted". The PR activity is being tracked under #91

@gautamrege gautamrege changed the title Some commits not appearing Checking SHA uniqueness in PR commits in case the branch has been deleted Sep 16, 2016
@apsc92
Copy link
Contributor

apsc92 commented Sep 20, 2016

@gautamrege I think it would be better if we change criteria and process of fetching commits of user.

Currently we have a criteria that either the repo or its origin(from which it is forked) should have minimum 25 stars.This approach somewhat defeats the purpose of open source if someone just do commits on its forked repo(i.e. do some customization for himself) and never sends a pr to the original repo.I think these commits should not be awarded points unless this forked repo too have min 25 stars.

We should have two cases in fetching commit

  • If the forked repo have 25 stars of its own then consider the commits done on it because having 25 stars make sure it is used by other people too.If a pr is sent to origin then we should not evaluate those commits(using sha) for whom points are already awarded.
  • If the forked repo doesn't have 25 stars then consider commits only via pr sent to origin.

Taking this new approach would solve the problem of considering commits even after the branch is deleted.

@gautamrege
Copy link
Member

@apsc92 How do we handle squashed commits? Ideally, when a PR is sent, sending 1 commit that squashes the rest is the recommended approach. In that case, whatever effort you put in will still fetch you at most 4 or 5 points.

Even if I have a forked repository, and I push commits to it regularly to beat the system by not ending a PR, there will be a time when you will be either caught or you are ruining your own public profile. Since, this approach will be taken by dredges of the community (< 0.01%) and I would prefer to trust the community for a good ethical effort.

Now, if your commits are scored on the fork with less than 25 stars, you send a PR or PR with squashed commits and even if the PR is rejected, you still get value for the effort you have put in. Hence we try to look at commits and not commits in a PR. What do you think?

THIS particular ticket is of the rare case of a branch being deleted and PR getting accepted before CodeCuriosity fetches them! This is rare but can be handled cleanly.

@apsc92
Copy link
Contributor

apsc92 commented Sep 20, 2016

@gautamrege Makes sense. For this particular problem one approach which comes in my mind is that we fetch commits of user from master branch of the origin once in a while and insert only unique commit on the bases of sha.
There might we a better way to do this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants