Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

gps: remove unused gopkgin code #1447

Closed
wants to merge 1 commit into from
Closed

gps: remove unused gopkgin code #1447

wants to merge 1 commit into from

Conversation

jmank88
Copy link
Collaborator

@jmank88 jmank88 commented Dec 12, 2017

What does this do / why do we need it?

This PR removes unused gopkgin code. Special case gopkgin types were used to augment normal git behavior, but they were being given standard github urls, so the special case code was not relevant. Now gopkgin imports map directly to their github source, avoiding duplication/isolation for each unique gopkgin import.

Which issue(s) does this PR fix?

Towards #1250 and #431

// this should only be reachable if there's an error in the regex
return nil, fmt.Errorf("could not parse %q as a gopkg.in major version", majorStr[1:])
}

mb := make(maybeSources, len(gopkginSchemes))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Why are we modifying a github url with gopkginSchemes?

Copy link
Member

Choose a reason for hiding this comment

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

It's supposed to be a conservative approach, mirroring gopkg.in's current behavior most closely: the real, running service supports only http and https, not ssh or git.

@jmank88
Copy link
Collaborator Author

jmank88 commented Dec 12, 2017

Codeclimate is a bit overzealous

@jmank88
Copy link
Collaborator Author

jmank88 commented Dec 26, 2017

It looks like the dead code was reactivated/fixed since the time I originally branched. Closing.

Still seems like there is an opportunity for better shared caching.

@jmank88 jmank88 closed this Dec 26, 2017
@sdboyer
Copy link
Member

sdboyer commented Dec 26, 2017

yeah, I was confused by this PR - I'm not sure what would have changed to either deactivate or reactivate it.

in any case, yes, there's certainly opportunity here for more sharing; it's always been the difficulty of maintaining thread safety and non-duplication guarantees while having to operate without the information yielded by the maybeSource.try() implementations...and the fact that the inputs (especially with gopkg.in's filtered version lists) can influence source return values.

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

Successfully merging this pull request may close these issues.

3 participants