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

Remove unnecessary assignment of Commit#repo #1508

Merged

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.

None yet

2 participants