Skip to content
This repository has been archived by the owner on Mar 1, 2019. It is now read-only.

Perform some housekeeping #95

Merged
merged 1 commit into from
Oct 6, 2017
Merged

Conversation

Xanewok
Copy link
Collaborator

@Xanewok Xanewok commented Oct 5, 2017

No description provided.

@@ -446,7 +446,7 @@ impl<L: AnalysisLoader> AnalysisHost<L> {
})
}

// e.g., https://github.com/rust-lang/rust/blob/master/src/libcollections/string.rs#L261-L263
// e.g., https://github.com/rust-lang/rust/blob/master/src/liballoc/string.rs#L261-L263
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed, since the previous link was invalid

},
// A crate we've never seen before.
None => {
// We have fresher data than what we can read.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Wanted to get rid of defining the same block twice for two cases, is this okay?

let mut buf = String::new();
file.read_to_string(&mut buf).unwrap();

let buf = read_file_contents(path).or_else(|err| {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Separated into read_file_contents to catch a possible error and report it, however if we were to ignore warning about the error, this could be further simplified with ok()?s - should I change that?

Copy link
Member

Choose a reason for hiding this comment

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

Nah, seems good to acknowledge the error

@@ -6,21 +6,17 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

macro_rules! option_try(
Copy link
Collaborator Author

@Xanewok Xanewok Oct 5, 2017

Choose a reason for hiding this comment

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

impl Try for Option has landed! 🎉

@nrc nrc merged commit c53f644 into rust-dev-tools:master Oct 6, 2017
@nrc
Copy link
Member

nrc commented Oct 6, 2017

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants