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

Decide whether internal changes count as breaking or not #13

Closed
lucaswerkmeister opened this issue Dec 4, 2021 · 3 comments
Closed
Milestone

Comments

@lucaswerkmeister
Copy link
Owner

So far, several releases have included a “BREAKING CHANGE (internal)”, which is a breaking change to the private methods used to communicate between Session and its subclasses. Before the 1.0.0 release, we need to figure out if those kinds of changes are really considered breaking and require a major version bump in future, or if this whole interface is considered unstable and can be changed with only a minor (or even patch?) version bump.

@lucaswerkmeister lucaswerkmeister added this to the 1.0.0 release milestone Dec 4, 2021
@lucaswerkmeister lucaswerkmeister changed the title Decide internal changes count as breaking or not Decide whether internal changes count as breaking or not Dec 4, 2021
@lucaswerkmeister
Copy link
Owner Author

Correction: at least internalGet and internalPost are currently jsdoc’ed as protected, not private. That lends at least some justification to the idea of considering them part of the stable interface.

@lucaswerkmeister
Copy link
Owner Author

I think one policy I could live with is that the protected interface can be changed with a minor release, just not with a patch release; libraries depending on this interface would be advised to use ~1.0.0 instead of ^1.0.0.

I should also double-check if internalGet and internalPost are the only parts of the protected interface, or if I’m missing something.

lucaswerkmeister added a commit that referenced this issue Sep 21, 2022
These will be part of the stable interface (cf. #13, #21), so they
should have some documentation. In DefaultUserAgentWarning, add params
like in the other classes so that we have something to document :)
lucaswerkmeister added a commit that referenced this issue Nov 12, 2022
As proposed in #13: we have a public stable interface and an internal
interface, and we’re allowed to change the internal interface between
minor versions, to leave ourselves some breathing room.

I’ll say here (but don’t consider it worth including in the README) that
I expect some level of common sense to be applied with regard to the
stable interface. For example, since the documentation “strongly
discourages” changing or removing DEFAULT_OPTIONS entries, I will not
consider changes to be incompatible just because they would
theoretically break a user who disregarded this advice.
@lucaswerkmeister
Copy link
Owner Author

Done with 43b8ad5.

lucaswerkmeister added a commit that referenced this issue Nov 12, 2022
This seems much nicer than appending another `authorization` parameter
(needed for #9) to the signature. The User-Agent header needs special
handling in fetch.js, but we can implement that by getting the user
agent out of the headers object (requiring all-lowercase so we don’t
have to deal with different-case versions).

In internalRequest(), we now have two “headers” variables, so to avoid
confusion let’s not only name the new one `requestHeaders` but also
rename the old one to `responseHeaders`.

Note that according to our new stable interface policy (#13), we’re
allowed to make internal breaking changes like with a minor version
bump, which pre-1.0 is any release. I think at the moment I’m inclined
to make the next release 0.7.2, not 0.8.0, since there will be no change
to normal package users.
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

No branches or pull requests

1 participant