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
Have and expose .Internal modules #211
Comments
I'd like to prioritize this issue since there's so much demand for access to the internals (see also #25, #103). What should the module hierarchy for the internals look like? How about
? If we want to expose the constructors in a PVP-versioned module as I proposed in #98 (comment), that could happen in |
I too am encountering a longing for more of the internals, often when I want some inner-loop code doing a specialized thing to be able to run quickly. For instance, today I wanted to be able to grab a list of the items and their hashes from a (Some fuller notes from me in #280, which was just a duplicate of this issue.) My take on exported vs hidden modules is that probably everything should be exported. I think it's fair game to have some of the "exported" modules be subject to arbitrary changes patch by patch, and it seems better to me to allow users this access (given that they know what they're getting into) than to attempt to gatekeep. That would suggest a fairly simple change in which all the modules become exposed, but some are considered internal and not public facing. Falling short of that, at least exposing the constructors would allow people to do what they need to. I do like the sound of #98 (comment), in which some subset of the internals (such as the data types) do have versioning guarantees. |
Thanks for the feedback, @jamespayor! :) I agree that it would be best and easiest to simply expose all the internal modules. I think we should expose them with different names though to make it clearer what's internal and what isn't. How about
? I believe that in order to avoid creating merge conflicts for the large number of open PRs, we should probably not move the existing internal modules, but instead add modules with the new names that reexport the existing internal modules. |
The word Base usually goes away in favor of Internal.
…On Thu, Jul 2, 2020, 7:54 AM Simon Jakobi ***@***.***> wrote:
Thanks for the feedback, @jamespayor <https://github.com/jamespayor>! :)
I agree that it would be best and easiest to simply expose all the
internal modules. I think we should expose them with different names though
to make it clearer what's internal and what isn't. How about
Data.HashMap.Internal <- Data.HashMap.Base # or Data.HashMap.Internal.Base?
Data.HashMap.Internal.Array <- Data.HashMap.Array
Data.HashMap.Internal.List <- Data.HashMap.List
Data.HashMap.Internal.Strict <- Data.HashMap.Strict.Base
Data.HashMap.Internal.Unsafe <- Data.HashMap.Unsafe
Data.HashSet.Internal <- Data.HashSet.Base
?
I believe that in order to avoid creating merge conflicts for the large
number of open PRs, we should probably not move the existing internal
modules, but instead *add* modules with the new names that reexport the
existing internal modules.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#211 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAOOF7OUTGM5T5MA73BHBJTRZRYQ3ANCNFSM4FDPHW6Q>
.
|
Cool - yeah renaming to @sjakobi fwiw, on my model of Git, if it's just renaming the files then the merge will likely go smoothly. If you do try renaming them, you could checkout the most complicated PR and verify that a merge or rebase works. If so, we get to avoid duplication in the file structure. |
This also removes some "Stability" annotations from internal modules. Context: #211.
This also removes some "Stability" annotations from internal modules. Context: #211.
#283 addressed the original request here. I'll make a separate issue to discuss exposing the core types and constructors from a PVP-versioned module. |
Actually I'd rather wait for user requests for this, so we get a better understanding of the requirements. |
containers
does it.Today, my co-worker had a problem to which my solution was
alterF
; unfortunately,alterF
is not yet in the released version. If the internals had been exposed, we could have backported the code from the github version without any version shenanigans; as it was, we had to accept the (probably minor) performance hit oflookup
+insert
due to the retraversal.The text was updated successfully, but these errors were encountered: