Skip to content

Conversation

@yakov116
Copy link
Member

@yakov116 yakov116 commented Jun 7, 2020

Fixes #2927

@fregante would love your input.

A bit of history of what i thought of doing.

First attempt was to get all the tag release dates in the original call. But github (as usual) does not give it 100% of the time.
So the next thing I did was if there was no date get it from the dom.
But then I realized that you can have commit dates not in order. (Very common by bootstrap on a rebase and merge).

So then I went with the following.

Try to get the ahead count from the dom. If the dom does not have it, take the release date from the dom and get the aheadcount from the API. With the issue of commit dates not in order, we should not be that off.

One issue, if the tag was created locally together with a commit the count will be ahead by one since they both happened the same time. I could fix it, not sure if its worth it.

Let me know what you think.

Test On https://github.com/fregante/delegate-it

@yakov116

This comment has been minimized.

@yakov116 yakov116 closed this Jun 7, 2020
@yakov116 yakov116 reopened this Jun 7, 2020
@fregante
Copy link
Member

fregante commented Jun 7, 2020

Two problems:

  • I can tag an old commit today. The commit’s date is useless
  • So much code

I think we should go with my suggestion: #2927 (comment)

This way we get the exact match if it's within the last 10/20/whatever commits, and if not, we don't have to guess, we just display "+".

The best part: it's just one API call. Total. This PR has 2 API calls and a fetchDom

@yakov116
Copy link
Member Author

yakov116 commented Jun 7, 2020

Agree

@yakov116 yakov116 closed this Jun 7, 2020
@yakov116 yakov116 deleted the latest-tag-from-api branch June 8, 2020 03:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

latest-tag-button sometimes fails getting the "ahead-by count"

2 participants