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

Rename exported structs and functions #63

Closed
wants to merge 3 commits into from
Closed

Rename exported structs and functions #63

wants to merge 3 commits into from

Conversation

samuong
Copy link
Owner

@samuong samuong commented May 3, 2021

This reduces the "public API" of the main package to nothing. The next
release of Alpaca will be v2.0.0, and any exported names will be made
available in sub-packages with a minor version bump.

@samuong
Copy link
Owner Author

samuong commented May 3, 2021

Most of the changes here are simple renamings of types and functions. The only exception is that I've also merged the PACData and pacData types. It felt a bit overkill to keep these separate, but @camh- let me know if you think there's a reason this shouldn't be merged?

@camh-
Copy link
Collaborator

camh- commented May 3, 2021

Could you create an issue for this an assign it the right number of story points. It's a little hard to track velocity and burndown without it.

The purpose of this change is to reduce code duplication in
`authenticator.go`. Previously, we needed to have two separate
implementations of the NTLM handshake: one for CONNECT requests (which
are written directly to a `net.Conn`) and another for all other requests
(which are sent via an `http.Transport`). Now that we only need one
(written against any `http.RoundTripper`), it will be simpler to
implement additional authentication protocols in the future, such as
Kerberos.
This reduces the "public API" of the `main` package to nothing. The next
release of Alpaca will be v2.0.0, and any exported names will be made
available in sub-packages with a minor version bump.
@samuong
Copy link
Owner Author

samuong commented May 3, 2021

I guess I wasn't very clear in my original description, but my plan for the future is to split the main package up into smaller packages (with more carefully designed APIs than what's there now). As I do this, I don't want to constantly break compatibility and have to bump the major version over and over again. So I thought it'd be easier to just break compatibility once, and start with a clean slate.

@camh-
Copy link
Collaborator

camh- commented May 3, 2021

So I thought it'd be easier to just break compatibility once, and start with a clean slate.

I don't think you'll be breaking compatibility at all, since it is all currently in package main (not importable as i understand it). You could just move things bit by bit and as you do that, only export what needs to be exported as you make packages out of things, but up to you.

@samuong samuong force-pushed the badproxy branch 2 times, most recently from f85c0d8 to b2901dd Compare May 3, 2021 12:28
@samuong
Copy link
Owner Author

samuong commented May 3, 2021

Ah, I hadn't ever actually thought about main not being importable, but that makes sense. Just checked and I get an error if I try, even if I import it under a different name. So we don't need this then, I'll abandon this change. Thanks!

@samuong samuong closed this May 3, 2021
@samuong samuong deleted the unexport branch May 3, 2021 12:30
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