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

IsStdlib() enhancement #9

Merged
merged 2 commits into from
Apr 6, 2016
Merged

IsStdlib() enhancement #9

merged 2 commits into from
Apr 6, 2016

Conversation

dAdAbird
Copy link
Contributor

@dAdAbird dAdAbird commented Apr 5, 2016

I've refactored a bit IsStdlib approach.

I try to get stdlib package's list from GOROOT (this is how go list std does). And switchs to your predefined list In case it fails.


var stdPkgs []string

var stdPkgsDedault = []string{
Copy link

Choose a reason for hiding this comment

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

There's a typo in the word "default" (here, and in 2 other places).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, thanks!

@dmitshur
Copy link

dmitshur commented Apr 6, 2016

I try to get stdlib package's list from GOROOT (this is how go list std does).

I'm not sure I understand, what is the benefit of doing that?

@dAdAbird
Copy link
Contributor Author

dAdAbird commented Apr 6, 2016

The benefit it that stdlib may change in future releases (for example "context" in 1.7 golang/go#14660), but current solution that based on predefined list doesn't follow this changes.

@divan divan merged commit 729f519 into divan:master Apr 6, 2016
@dAdAbird dAdAbird deleted the is-stdlib branch April 6, 2016 12:40
@divan
Copy link
Owner

divan commented Apr 6, 2016

Amazing, thanks!
@shurcooL, yes, that was in FIXME comments - I want code to work with future versions of Go without modification, even if stdlib will be changed drastically.

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.

3 participants