Skip to content

Improve repo-age’s reliability#3201

Merged
fregante merged 10 commits intorefined-github:masterfrom
yakov116:repo-age-api
Jun 10, 2020
Merged

Improve repo-age’s reliability#3201
fregante merged 10 commits intorefined-github:masterfrom
yakov116:repo-age-api

Conversation

@yakov116
Copy link
Member

@yakov116 yakov116 commented Jun 8, 2020

@fregante fregante closed this Jun 8, 2020
@fregante fregante reopened this Jun 8, 2020
yakov116 and others added 2 commits June 8, 2020 13:37
Co-authored-by: Fregante <opensource@bfred.it>
Co-authored-by: Fregante <opensource@bfred.it>
@fregante fregante changed the title Use API instead of fetchDom for repo-age Improve repo-age’s reliability Jun 8, 2020
Co-authored-by: Fregante <opensource@bfred.it>
yakov116 and others added 2 commits June 8, 2020 17:18
Co-authored-by: Flo Edelmann <florian-edelmann@online.de>
const timeStamp = select('relative-time', commit)!.attributes.datetime.value;
const {pathname} = select<HTMLAnchorElement>('a.message', commit)!;
return [timeStamp, pathname];
return getRepoAge(commitSha, commitsCount - Math.min(6, commitsCount));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this mean? That's 6?

Copy link
Member Author

@yakov116 yakov116 Jun 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want all from the last commit - 6 (Which is 5 its complicated but I thnk 0 and 1 are the same)
I will double check give me a min

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes 0 and 1 are the same

Try it move the 1886 to 1887 and 1888

{
  repository(owner: "sindresorhus", name: "refined-github") {
    defaultBranchRef {
      target {
        ... on Commit {
          history(first: 5, after: "208835085ee2a78b6a9d4f30b1407c7eac1b88b3 1886") {
            nodes {
              authoredDate
              committedDate
              resourcePath
            }
          }
        }
      }
    }
  }
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why 6?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want 5, 1 =0 (I think)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK the math is like this. We are at commit 10 and we want AFTER commit 10 so we need to start from number 9 = (10 - (1 + 5) )

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the math belongs to getRepoAge, not outside.
5 is inside and 6 is outside; doesn't make sense.

Suggested change
return getRepoAge(commitSha, commitsCount - Math.min(6, commitsCount));
return getRepoAge(commitSha, commitsCount);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have commit number 1 we want the last 5. There are 1000 commits. If we do 1 + 1000 (The commit count) we get commit number 1001 which does not exist.
So we need to go back 1 and then another 5 to get the last 5.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The latest version looks right

@fregante
Copy link
Member

I think in the Repository refresh, this information should be moved to the sidebar:

@yakov116
Copy link
Member Author

I'll leave that for @FloEdelmann. I promise he will do a better job 🥇.

@fregante fregante merged commit 77bbcbc into refined-github:master Jun 10, 2020
@fregante fregante added the bug label Jun 10, 2020
@yakov116 yakov116 deleted the repo-age-api branch June 10, 2020 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

AngularJS repository marked as being 51 years old

3 participants