Skip to content

Remove Text.Parsing module prefix #78

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

Closed
thomashoneyman opened this issue Feb 2, 2022 · 4 comments · Fixed by #89
Closed

Remove Text.Parsing module prefix #78

thomashoneyman opened this issue Feb 2, 2022 · 4 comments · Fixed by #89
Labels
type: breaking change A change that requires a major version bump.

Comments

@thomashoneyman
Copy link
Contributor

To go along with purescript-contrib/purescript-parsing#147 -- this library could just as well do with just using StringParser as the module prefix, rather than including Text.Parsing, as we don't follow the Haskell example of putting all text-related packages under a Text grouping.

@thomashoneyman thomashoneyman added the type: breaking change A change that requires a major version bump. label Feb 2, 2022
@chtenb
Copy link
Member

chtenb commented Feb 18, 2022

While we're at it we might as well update the module structure a bit. Since this is a very simple package I think it would make sense to have a single module act as the main API, reexporting from Combinators and CodePoints, similarly to purescript-strings (see https://github.com/purescript/purescript-strings/blob/master/src/Data/String.purs)

@chtenb
Copy link
Member

chtenb commented Feb 26, 2022

@thomashoneyman What is the attitude towards these kind of changes in the PureScript ecosystem? Should we just do this the next time we push a breaking change? This would be the next release.

@thomashoneyman
Copy link
Contributor Author

thomashoneyman commented Feb 26, 2022

@chtenb Core libraries avoid breaking changes, and so do particularly widely used contrib libraries (Aff, Argonaut), but otherwise it’s up to the maintainers to decide when a change is worth breaking things.

When breaking things we generally try to make sure the upgrade path is well-described. This change is a simple find-replace so that’s easy — unless another package in the package set is using the same module names. Shuffling the modules around is a bigger change, so I hope we can still make it easy to migrate. I suppose that would just be changing all your imports to one single import though, so that’s easy enough.

@chtenb
Copy link
Member

chtenb commented Feb 26, 2022

Is there a particular reason we don't use the Text namespace anymore by the way? Or put another way, do we have a formal set of guidelines about module names somewhere?

chtenb added a commit that referenced this issue Mar 7, 2022
@chtenb chtenb closed this as completed in #89 Mar 8, 2022
chtenb added a commit that referenced this issue Mar 8, 2022
…per #78 (#89)

* Move existing StringParser module to Parser

* Re-export other modules in main API

* Remove Text.Parsing prefex per #78
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: breaking change A change that requires a major version bump.
2 participants