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

Fork is not more recent #13

Closed
Mottie opened this Issue Feb 19, 2016 · 7 comments

Comments

Projects
None yet
2 participants
@Mottie

Mottie commented Feb 19, 2016

I found an example where a flame icon is added to a fork which appears to be exactly the same as the source...

Source: https://github.com/cahnory/jQuery.keyboard
Flamey Fork: https://github.com/behnam/jQuery.keyboard

There are only 2 commits in each repo.

@musically-ut

This comment has been minimized.

Show comment
Hide comment
@musically-ut

musically-ut Feb 19, 2016

Owner

Interesting. Thanks for bringing this example to my notice. I will investigate it.

Owner

musically-ut commented Feb 19, 2016

Interesting. Thanks for bringing this example to my notice. I will investigate it.

musically-ut added a commit that referenced this issue Feb 21, 2016

Use the 'pushed_at' time for current repo
instead of using the last 'committed_at' time. This is because we use
the 'pushed_time' of the fork to determine if the fork is newer than the
current repository.

This should address #13.j
@musically-ut

This comment has been minimized.

Show comment
Hide comment
@musically-ut

musically-ut Feb 23, 2016

Owner

It turns out that the pushed_at field returned by the API is bit weirder than I had anticipated.

At the moment, I take the fork's pushed_at value and compare it with the current repository's last commit. That is an approximation at best and I think until Github clarifies what the exact meaning of the fields 'updated_at' and 'pushed_at' is, we'll just have to make the best of the information we have. :(

The alternate is to get commits for both the source and the destination repository which would take the API call count from 2 to 3 per repo. I would be more comfortable going that route when I allow users to authenticate via the options page to avoid rate-limiting.


To address it, I will add a "may" in the explanation for the flame icon and keep this issue open, in case Github clarifies the meanings or I add authentication.

Owner

musically-ut commented Feb 23, 2016

It turns out that the pushed_at field returned by the API is bit weirder than I had anticipated.

At the moment, I take the fork's pushed_at value and compare it with the current repository's last commit. That is an approximation at best and I think until Github clarifies what the exact meaning of the fields 'updated_at' and 'pushed_at' is, we'll just have to make the best of the information we have. :(

The alternate is to get commits for both the source and the destination repository which would take the API call count from 2 to 3 per repo. I would be more comfortable going that route when I allow users to authenticate via the options page to avoid rate-limiting.


To address it, I will add a "may" in the explanation for the flame icon and keep this issue open, in case Github clarifies the meanings or I add authentication.

@Mottie

This comment has been minimized.

Show comment
Hide comment
@Mottie

Mottie Feb 23, 2016

It looks like you want the pushed_at value:

Source (cahnory)

"created_at": "2011-10-28T19:06:59Z",
"updated_at": "2016-01-21T18:57:06Z",
"pushed_at": "2011-10-29T10:02:52Z",

Fork (behnam)

"created_at": "2012-01-02T04:37:08Z",
"updated_at": "2013-01-05T18:04:41Z",
"pushed_at": "2011-10-29T10:02:52Z",

Fork (unwelt)

"created_at": "2016-02-09T02:09:55Z",
"updated_at": "2016-02-09T02:09:56Z",
"pushed_at": "2016-02-09T05:42:34Z",

So it looks like the "unwelt" branch has more recent updates, but no stars. Looking at the network graph, it's obvious that unwelt has the most updates. I don't even see behnam's fork there.

So I'm guessing the pushed_at is the value you want to use. I'm not sure what the updated_at value represents, unless it refers to some unseen activity... no issues, recent comments, PRs or wiki changes match the source updateed_at time that I can find.

I wonder if we need two different labels for forks:

  • Most stars
  • Most recent update

Mottie commented Feb 23, 2016

It looks like you want the pushed_at value:

Source (cahnory)

"created_at": "2011-10-28T19:06:59Z",
"updated_at": "2016-01-21T18:57:06Z",
"pushed_at": "2011-10-29T10:02:52Z",

Fork (behnam)

"created_at": "2012-01-02T04:37:08Z",
"updated_at": "2013-01-05T18:04:41Z",
"pushed_at": "2011-10-29T10:02:52Z",

Fork (unwelt)

"created_at": "2016-02-09T02:09:55Z",
"updated_at": "2016-02-09T02:09:56Z",
"pushed_at": "2016-02-09T05:42:34Z",

So it looks like the "unwelt" branch has more recent updates, but no stars. Looking at the network graph, it's obvious that unwelt has the most updates. I don't even see behnam's fork there.

So I'm guessing the pushed_at is the value you want to use. I'm not sure what the updated_at value represents, unless it refers to some unseen activity... no issues, recent comments, PRs or wiki changes match the source updateed_at time that I can find.

I wonder if we need two different labels for forks:

  • Most stars
  • Most recent update
@musically-ut

This comment has been minimized.

Show comment
Hide comment
@musically-ut

musically-ut Feb 23, 2016

Owner

Thanks for investigating it. :)

Yes, "pushed_at" does work in this case, and there is a commit in lovely forks to use this.

However, during testing, I figured out that it doesn't work for other repositories. For example, for jaz303/tipsy, which is an example in the README, we see the following:

  "created_at": "2008-06-08T18:48:00Z",
  "updated_at": "2016-02-22T14:47:16Z",
  "pushed_at": "2015-12-15T05:55:21Z",

And for the best fork CloCkWeRX/tipsy, we see the following:

  "created_at": "2013-01-23T01:15:11Z",
  "updated_at": "2016-02-15T22:27:02Z",
  "pushed_at": "2015-01-02T03:15:48Z",

Here, the pushed_at leads one to believe that the repositories match up. However, the fork actually contains many more commits than the origin, which hasn't been touched in the past 2 years.

I asked @GithubEngg on Twitter about this, presenting a hypothesis, but haven't heard back from them.

I may write have written to Ivan, who answered this question on SO a while back. However, that information seems to be incomplete/outdated now.

Owner

musically-ut commented Feb 23, 2016

Thanks for investigating it. :)

Yes, "pushed_at" does work in this case, and there is a commit in lovely forks to use this.

However, during testing, I figured out that it doesn't work for other repositories. For example, for jaz303/tipsy, which is an example in the README, we see the following:

  "created_at": "2008-06-08T18:48:00Z",
  "updated_at": "2016-02-22T14:47:16Z",
  "pushed_at": "2015-12-15T05:55:21Z",

And for the best fork CloCkWeRX/tipsy, we see the following:

  "created_at": "2013-01-23T01:15:11Z",
  "updated_at": "2016-02-15T22:27:02Z",
  "pushed_at": "2015-01-02T03:15:48Z",

Here, the pushed_at leads one to believe that the repositories match up. However, the fork actually contains many more commits than the origin, which hasn't been touched in the past 2 years.

I asked @GithubEngg on Twitter about this, presenting a hypothesis, but haven't heard back from them.

I may write have written to Ivan, who answered this question on SO a while back. However, that information seems to be incomplete/outdated now.

@musically-ut

This comment has been minimized.

Show comment
Hide comment
@musically-ut

musically-ut Mar 5, 2016

Owner

With Ivan's input, I have switched to an heuristic based on Compare API. I am testing it locally and hope to release it to Chrome store/Firefox addons soon.

Owner

musically-ut commented Mar 5, 2016

With Ivan's input, I have switched to an heuristic based on Compare API. I am testing it locally and hope to release it to Chrome store/Firefox addons soon.

@musically-ut

This comment has been minimized.

Show comment
Hide comment
@musically-ut

musically-ut Mar 6, 2016

Owner

@Mottie I released the extension(s) earlier today. The Chrome extension update is live while the Firefox addon will take a while to get a preliminary review and then some to get to the users.

Could you close the issue after confirming that it works?

Thanks!

Owner

musically-ut commented Mar 6, 2016

@Mottie I released the extension(s) earlier today. The Chrome extension update is live while the Firefox addon will take a while to get a preliminary review and then some to get to the users.

Could you close the issue after confirming that it works?

Thanks!

@Mottie

This comment has been minimized.

Show comment
Hide comment
@Mottie

Mottie Mar 6, 2016

It's looking good! Thanks for all your hard work!

Mottie commented Mar 6, 2016

It's looking good! Thanks for all your hard work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment