Skip to content

Conversation

@tniessen
Copy link
Contributor

@tniessen tniessen commented Jun 6, 2018

Repository#getCommit uses Commit.lookup and then manually assigns to Commit#repo even though Commit.lookup already does that. There is no way for Commit.lookup to return a Commit without a repo property.

@tniessen
Copy link
Contributor Author

Ping @tbranyen, @johnhaley81, @maxkorp.

@tniessen
Copy link
Contributor Author

It's been far more than a year, please close this if you do not want to merge it.

@tbranyen
Copy link
Member

Tests aren't passing and the CI is failing. We can't merge it like this. Please correct the issues or ask for help and we can get this merged if it's still needed.

@tniessen
Copy link
Contributor Author

@tbranyen If I remember correctly, the failures were due to problems on your side, not on mine, and CI last ran two years ago on this. But sure, I will rebase and force-push to cause a new CI run.

Repository#getCommit uses Commit.lookup and then manually assigns
to Commit#repo even though Commit.lookup already does that.
@tniessen tniessen force-pushed the repository-remove-unnecessary-assignment branch from 112317f to c2b61a2 Compare January 19, 2020 18:47
@tniessen
Copy link
Contributor Author

tniessen commented Jan 19, 2020

My patch seems to be working fine, so it must have been problems with your existing code or your CI setup.

@tbranyen
Copy link
Member

Looks good 👍, thanks for getting the PR into a mergeable state.

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.

2 participants