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

Make rustdoc not include self-by-value methods from Deref target #45645

Merged
merged 1 commit into from Nov 20, 2017

Conversation

Projects
None yet
7 participants
@fhartwig
Copy link
Contributor

fhartwig commented Oct 31, 2017

Fixes #39550

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Oct 31, 2017

r? @frewsxcv

(rust_highfive has picked a reviewer for you, use r? to override)

@kennytm

This comment has been minimized.

Copy link
Member

kennytm commented Oct 31, 2017

Two rustdoc test cases need to be updated, removing the count_ones methods. Or maybe rustdoc should keep the self-by-value methods if the target is Copy (but that would get us into #33396).

[00:56:43] ---- [rustdoc] rustdoc/issue-19190-3.rs stdout ----
[00:56:43] 	
[00:56:43] error: htmldocck failed!
[00:56:43] status: exit code: 1
[00:56:43] command: "/usr/bin/python2.7" "/checkout/src/etc/htmldocck.py" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc/issue-19190-3.stage2-x86_64-unknown-linux-gnu" "/checkout/src/test/rustdoc/issue-19190-3.rs"
[00:56:43] stdout:
[00:56:43] ------------------------------------------
[00:56:43] 
[00:56:43] ------------------------------------------
[00:56:43] stderr:
[00:56:43] ------------------------------------------
[00:56:43] 20: @has check failed
[00:56:43] 	`XPATH PATTERN` did not match
[00:56:43] 	// @has - '//*[@id="method.count_ones"]' 'fn count_ones(self) -> u32'
[00:56:43] 
[00:56:43] Encountered 1 errors
[00:56:43] 
[00:56:43] ------------------------------------------
[00:56:43] 
[00:56:43] failures:
[00:56:43]     [rustdoc] rustdoc/issue-19190-2.rs
[00:56:43]     [rustdoc] rustdoc/issue-19190-3.rs
@bors

This comment has been minimized.

Copy link
Contributor

bors commented Nov 1, 2017

☔️ The latest upstream changes (presumably #45187) made this pull request unmergeable. Please resolve the merge conflicts.

@frewsxcv

This comment has been minimized.

Copy link
Member

frewsxcv commented Nov 5, 2017

@QuietMisdreavus

This comment has been minimized.

Copy link
Member

QuietMisdreavus commented Nov 7, 2017

What i would suggest to fix the tests, rather than trying to wrangle with Copy, is to instead change the Derefs in those tests to point to String instead, and use fn as_str(&self) -> &str and fn new() -> String instead. That way the test is still checking for the proper thing. Note that for issue-19190-3.rs the Deref impls are actually in src/test/rustdoc/auxiliary/issue-19190-3.rs - that test is checking for cross-crate imports.

@shepmaster

This comment has been minimized.

Copy link
Member

shepmaster commented Nov 11, 2017

We haven't heard from you in a week or so, @fhartwig — will you have time to address the most recent feedback?

@fhartwig

This comment has been minimized.

Copy link
Contributor Author

fhartwig commented Nov 11, 2017

@shepmaster Sorry for the silence, I've been pretty busy. I should be able to finish this in the next 2-3 days.

@fhartwig fhartwig force-pushed the fhartwig:39550 branch from d75657a to 32af136 Nov 19, 2017

@fhartwig

This comment has been minimized.

Copy link
Contributor Author

fhartwig commented Nov 19, 2017

I've rebased the changes and adapted the failing tests as @QuietMisdreavus suggested (thanks!).

Sorry for the delay.

@QuietMisdreavus

This comment has been minimized.

Copy link
Member

QuietMisdreavus commented Nov 20, 2017

@bors r+

Thanks!

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Nov 20, 2017

📌 Commit 32af136 has been approved by QuietMisdreavus

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Nov 20, 2017

⌛️ Testing commit 32af136 with merge e061383...

bors added a commit that referenced this pull request Nov 20, 2017

Auto merge of #45645 - fhartwig:39550, r=QuietMisdreavus
Make rustdoc not include self-by-value methods from Deref target

Fixes #39550
@bors

This comment has been minimized.

Copy link
Contributor

bors commented Nov 20, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: QuietMisdreavus
Pushing e061383 to master...

@bors bors merged commit 32af136 into rust-lang:master Nov 20, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.