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 Type::detect_has_vtable_cycle. #787

Merged
merged 1 commit into from
Jul 5, 2017

Conversation

SirVer
Copy link
Contributor

@SirVer SirVer commented Jul 4, 2017

After some discussion in #765 we do not think anymore that this can ever be true.

@SirVer
Copy link
Contributor Author

SirVer commented Jul 4, 2017

r? @fitzgen

@emilio
Copy link
Contributor

emilio commented Jul 4, 2017

Can you squash the commits please? Thanks!

After some discussion in rust-lang#765 we do not think anymore this it can ever
be true.
@SirVer SirVer force-pushed the 00_fix_repetition_operation branch from 1850ba8 to 672b740 Compare July 4, 2017 20:14
@SirVer
Copy link
Contributor Author

SirVer commented Jul 4, 2017

@emilio Done.

My 2c: In my humble opinion this should be done by the merge bot. It is quite useful to keep the history of the commits in the related PR to see the development of the discussion and GitHub does not deal well with rebasing.

GitHubs own merge tools actually allow for that - squashing everything into one commit on merge. The mergebot could do that too - the only question is which commit message to use. In my projects the tradition is to use the OP as the commit message and edit it to always reflect the state of the PR.

@fitzgen
Copy link
Member

fitzgen commented Jul 5, 2017

@bors-servo r=emilio

@bors-servo
Copy link

📌 Commit 672b740 has been approved by emilio

@bors-servo
Copy link

⌛ Testing commit 672b740 with merge 8f756be...

bors-servo pushed a commit that referenced this pull request Jul 5, 2017
Remove Type::detect_has_vtable_cycle.

After some discussion in #765 we do not think anymore that this can ever be true.
@bors-servo
Copy link

☀️ Test successful - status-travis
Approved by: emilio
Pushing 8f756be to master...

@bors-servo bors-servo merged commit 672b740 into rust-lang:master Jul 5, 2017
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.

5 participants