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

Don't fetch a diff on first commit #98

Merged
merged 1 commit into from
Jun 13, 2017
Merged

Don't fetch a diff on first commit #98

merged 1 commit into from
Jun 13, 2017

Conversation

bradleyfalzon
Copy link
Owner

Currently, the first commit has the latest commit with a relative
reference of number of commits being pushed. For example, if the
first event has just 1 commit at sha abcdef, commitFrom is abcdef~1
and commitTo is abcdef.

We use relative references to ensure we can always fetch the diff
even if the oldest commit has been removed (via push). But the first
commit doesn't appear to have a valid diff from any GitHub APIs so
to indicate this, we'll set commitFrom to blank.

When it comes to fetching the diff via the VCSReader interface, if
commitFrom is blank, return a nil io.ReadCloser and nil error.

The frontends already know how to handle lack of a diff, it just
occurs even when there's no error now.

This also relates to #79, which was handling fetching diffs when
the before ref no longer exists, which is the case when a branch
tracks a new tree or a new repository is created. I couldn't
combine the logic much further, because GitHub has no indication
when the commit follows a new tree (the before ref is from the
previous tree, whereas in a new repository it's all 0s).

So I wasn't able to combine any logic between this and #79, but
the integration test was renamed from new-go-repository to new-tree
to better reflect what it was doing.

Fixes #96.
Relates to #79.

Currently, the first commit has the latest commit with a relative
reference of number of commits being pushed. For example, if the
first event has just 1 commit at sha abcdef, commitFrom is abcdef~1
and commitTo is abcdef.

We use relative references to ensure we can always fetch the diff
even if the oldest commit has been removed (via push). But the first
commit doesn't appear to have a valid diff from any GitHub APIs so
to indicate this, we'll set commitFrom to blank.

When it comes to fetching the diff via the VCSReader interface, if
commitFrom is blank, return a nil io.ReadCloser and nil error.

The frontends already know how to handle lack of a diff, it just
occurs even when there's no error now.

This also relates to #79, which was handling fetching diffs when
the before ref no longer exists, which is the case when a branch
tracks a new tree or a new repository is created. I couldn't
combine the logic much further, because GitHub has no indication
when the commit follows a new tree (the before ref is from the
previous tree, whereas in a new repository it's all 0s).

So I wasn't able to combine any logic between this and #79, but
the integration test was renamed from new-go-repository to new-tree
to better reflect what it was doing.

Fixes #96.
Relates to #79.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 60.327% when pulling 788e00a on first-commit into 9e35dc9 on master.

@bradleyfalzon bradleyfalzon merged commit 367580f into master Jun 13, 2017
@bradleyfalzon bradleyfalzon deleted the first-commit branch June 13, 2017 05:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants