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

Only import implementations data from non-api crates #70

Closed
wants to merge 3 commits into from
Closed

Only import implementations data from non-api crates #70

wants to merge 3 commits into from

Conversation

jonasbb
Copy link
Contributor

@jonasbb jonasbb commented Jun 13, 2017

Addresses some of the concerns discussed here:
#49

Addresses some of the concerns discussed here:
#49
Copy link
Member

@nrc nrc left a comment

Choose a reason for hiding this comment

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

Instead of filtering here, could we read in all impls, but when requesting impls, only return non-api crate ones? Reasoning is that rustdoc will want impls for the api crates

jonasbb added 2 commits June 14, 2017 22:10
This is a breaking change to the API, thus increasing the version
number.
@jonasbb
Copy link
Contributor Author

jonasbb commented Jun 14, 2017

It is now possible to perform the filtering while retrieving the data. I even made the filtering option. Although this is now an API breaking change.

Copy link
Member

@nrc nrc left a comment

Choose a reason for hiding this comment

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

Thanks for the change, looks good. I had one more comment intline

@@ -568,13 +579,19 @@ pub struct PerCrateAnalysis {
def_names: HashMap<String, Vec<Id>>,
ref_spans: HashMap<Id, Vec<Span>>,
globs: HashMap<Span, Glob>,
impls: HashMap<Id, Vec<Span>>,
impls: HashMap<Id, Vec<SpanApi>>,
Copy link
Member

Choose a reason for hiding this comment

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

Could we add a single bool to the crate, rather than one to each span? If not, I'd prefer to use a tuple here rather than add another struct

@nrc nrc closed this Aug 5, 2017
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