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

WIP: Remove 'regex-posix' dependency #289

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tom-audm
Copy link

@tom-audm tom-audm commented Sep 9, 2019

WIP, please don't merge yet!

@tom-audm tom-audm mentioned this pull request Sep 9, 2019
2 tasks
@tom-audm
Copy link
Author

tom-audm commented Sep 9, 2019

Reasons for removing regex-posix:

  • It's an additional couple of transitive dependencies which don't add much functionality (not used pervasively throughout snap-core)
  • It's a dependency that's pretty orthogonal to snap-core's purpose
  • It currently has no active maintainer, doesn't work with GHC 8.8, etc. https://mail.haskell.org/pipermail/libraries/2019-September/029927.html
  • Its only current source form (besides Hackage) is a circa-2012 Sourceforge page

One solution is to, in addition to the work here, move assertBodyContains to the snap package, which is more batteries-included.

@tom-audm
Copy link
Author

tom-audm commented Sep 9, 2019

As described in the docs, this change requires a user who truly does want to use regexes to write:

import Text.Regex.Posix
assertBodyMatches "^Hello" (=~ "^Hello") r

instead of

assertBodyContains "^Hello" r

@mightybyte
Copy link
Member

I'm broadly in favor of this kind of dependency pruning. The API breakage gives me pause. But it also doesn't seem like a super commonly used function. Pinging @gregorycollins.

@hvr
Copy link
Member

hvr commented Sep 22, 2019

@tom-audm fwiw, the 3rd and 4th bullet points are currently being addressed... ;-)

@gregorycollins
Copy link
Member

It will break test code, but I'm probably okay with it? We'll need a major version bump unfortunately

@dten
Copy link
Contributor

dten commented Sep 15, 2020

Just chiming in here because I wanted to build more conveniently on windows. Have you considered replacing the regex-posix dep with regex-tdfa which has no C deps
(though i'm all for removing it entirely)

@gregorycollins
Copy link
Member

I think the consensus is that we're just going to get rid of this dependency (and the function) altogether

@mightybyte
Copy link
Member

Yeah, I'm in favor of this change.

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.

5 participants