Seal the HasContext trait #280
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In the current iteration of this crate, a new breaking change is needed every time a method is added to the `HasContext` trait. This is because a downstream crate could theoretically provide its own implementation of `HasContext`, and would need to accommodate for these new methods. However, most users of `glow` do not implement the `HasContext` trait and only act as consumers of it. This means that downstream crates have to constantly update `glow` to take advantage of new updates, which can be annoying. This commit makes it so it is impossible to implement the `HasContext` trait outside of this crate. It does this by making a crate-private `Sealed` trait that is needed for implementors of `HasContext`. This makes it so methods can be freely added to `HasContext` without needing to release a new breaking version of `glow`. The goal of this change is to make `glow` v1.0.0 possible. With this change it should be possible to release a stable API, assuming none of the other APIs need to be changed or removed in any way. The downside of this change is that it becomes impossible to implement `HasContext` outside of this crate. I would be genuinely shocked if any such implementations existed. Signed-off-by: John Nunley <[email protected]>
|
macOS failure appears to be a spurious network error |
|
@grovesNL When do you think you'll be able to review this? |
grovesNL
approved these changes
Feb 21, 2024
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the current iteration of this crate, a new breaking change is needed
every time a method is added to the
HasContexttrait. This is becausea downstream crate could theoretically provide its own implementation of
HasContext, and would need to accommodate for these new methods.However, most users of
glowdo not implement theHasContexttraitand only act as consumers of it. This means that downstream crates have
to constantly update
glowto take advantage of new updates, which canbe annoying.
This commit makes it so it is impossible to implement the
HasContexttrait outside of this crate. It does this by making a crate-private
Sealedtrait that is needed for implementors ofHasContext. Thismakes it so methods can be freely added to
HasContextwithout needingto release a new breaking version of
glow.The goal of this change is to make
glowv1.0.0 possible. With thischange it should be possible to release a stable API, assuming none of
the other APIs need to be changed or removed in any way.
The downside of this change is that it becomes impossible to implement
HasContextoutside of this crate. I would be genuinely shocked if anysuch implementations existed