Skip to content

Remove unused code #558

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

Merged
merged 17 commits into from
Feb 5, 2020
Merged

Remove unused code #558

merged 17 commits into from
Feb 5, 2020

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Jan 9, 2020

No description provided.

@jyn514 jyn514 force-pushed the remove-unused-code branch from 9ad0f91 to 729cb76 Compare January 11, 2020 19:50
@jyn514 jyn514 force-pushed the remove-unused-code branch from 27421e7 to 7cb0a07 Compare January 23, 2020 01:58
@jyn514
Copy link
Member Author

jyn514 commented Jan 23, 2020

r? @koenaad

There's lots of messing with pub in the PR, but what I actually care about is getting rid of the unused functions in utils/copy.rs and utils/rustc_version.rs. Getting rid of pub just made the compiler give me warnings so I could find them.

@yvrhdn
Copy link

yvrhdn commented Feb 2, 2020

Oh hey, sorry @jyn514. I didn't see this mention between all my Github notifications. I should probably tweak my github settings a bit...
I might be able to look at this tomorrow, otherwise Tuesday should work.

Copy link

@yvrhdn yvrhdn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work cleaning up some technical debt! I have added some comments here and there for small improvements but this looks good to me 🙂

@jyn514
Copy link
Member Author

jyn514 commented Feb 5, 2020

Addressed review comments

@yvrhdn
Copy link

yvrhdn commented Feb 5, 2020

Cool! Looks good to me. Again, great work making the docs.rs codebase a bit easier to work with 😄

@jyn514 jyn514 merged commit 3809f94 into rust-lang:master Feb 5, 2020
@jyn514 jyn514 deleted the remove-unused-code branch February 5, 2020 12:45
jyn514 added a commit that referenced this pull request Feb 6, 2020
This reverts commit 3809f94.

This caused all builds to fail in the last step when copying
documentation into the database.
jyn514 added a commit that referenced this pull request Feb 6, 2020
This reverts commit 3809f94.

This caused all builds to fail in the last step when copying
documentation into the database.
jyn514 added a commit to jyn514/docs.rs that referenced this pull request Feb 6, 2020
* remove copy_dir

* make utils pub(crate) where possible

* remove dead code in rustc version

* make more things pub(crate)

* only compile ApplyMode::Temporary in test mode

* remove unused argument

* remove unused `handle_html` argument

* don't export metadata

* remove `pub` where possible

* remove unused import

* make create_pool pub(crate)

* fix bad merge

* pub(crate) doesn't make sense in lib.rs

Co-Authored-By: Koenraad Verheyden <mail@koenraadverheyden.com>

* address review comments re: copy test

* remote system dependencies from metadata

this was causing confusion, see rust-lang#164

* re-comment failing test

* fix failing test

Co-authored-by: Koenraad Verheyden <mail@koenraadverheyden.com>
jyn514 added a commit that referenced this pull request Feb 8, 2020
* remove copy_dir

* make utils pub(crate) where possible

* remove dead code in rustc version

* make more things pub(crate)

* only compile ApplyMode::Temporary in test mode

* remove unused argument

* remove unused `handle_html` argument

* don't export metadata

* remove `pub` where possible

* remove unused import

* make create_pool pub(crate)

* fix bad merge

* pub(crate) doesn't make sense in lib.rs

Co-Authored-By: Koenraad Verheyden <mail@koenraadverheyden.com>

* address review comments re: copy test

* remote system dependencies from metadata

this was causing confusion, see #164

* re-comment failing test

* fix failing test

Co-authored-by: Koenraad Verheyden <mail@koenraadverheyden.com>
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