Skip to content

Conversation

@flip111
Copy link
Contributor

@flip111 flip111 commented Apr 4, 2020

  • Levenshtein distance
  • Sørensen–Dice coefficient

Don't know how documentation update works.

* Levenshtein distance
* Sørensen–Dice coefficient
@thomashoneyman
Copy link
Contributor

thomashoneyman commented Apr 8, 2020

Hi @flip111! Thanks for adding these. I'm happy to add in the implementations, but I'm not quite ready to move the project over to Spago (we're going to need to move all the contributor libraries at some point, and I'd like to do them at once so that we can set up automatic publishing of new releases and that sort of thing).

To upload new documentation, see the publishing section in the Pulp docs.

With that in mind, could you revert the Spago parts of this change and remain on Bower for the dependencies?

@flip111
Copy link
Contributor Author

flip111 commented Apr 8, 2020

we're going to need to move all the contributor libraries at some point, and I'd like to do them at once so that we can set up automatic publishing of new releases and that sort of thing

I don't know what this means. But i get that you want to do stuff before adding this to spago.

I added spago because i use these two functions myself and i use spago for my project so i like this package to be on spago. What would be the difficulty adding it to spago now and then later do these other things?

@flip111
Copy link
Contributor Author

flip111 commented Apr 8, 2020

So you want packages.dhall and spago.dhall removed ?

@thomashoneyman
Copy link
Contributor

thomashoneyman commented Apr 8, 2020

I added spago because i use these two functions myself and i use spago for my project so i like this package to be on spago. What would be the difficulty adding it to spago now and then later do these other things?

This package is already in the official package sets (see: https://github.com/purescript/package-sets/blob/e689c7da77b3b0589eb8afd52d905099dd09dfad/src/groups/purescript-contrib.dhall#L208), so you can install it with spago install strings-extra.

I just don't want to switch the code here to use Spago as the dependency manager because we are going to switch purescript-contrib projects to use Spago as their dependency manager in the near-ish future, and I'd like to switch them all at the same time. We're going to have to switch things like our publishing workflow when we do that, and it's just easier to do it all at once (as a maintainer).

But that doesn't affect anyone installing this library into their project -- they can still do that with Spago no problem. It's purely an internal thing.

So you want packages.dhall and spago.dhall removed ?

Yes, thank you!

@thomashoneyman
Copy link
Contributor

Looks like the CI failure is from a missing dependency (in Bower):

$ npm run -s test

* Building project in /home/travis/build/purescript-contrib/purescript-strings-extra

[1/1 ModuleNotFound] test/Main.purs:8:1

  8  import Data.Number.Approximate ((≅))
     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

  Module Data.Number.Approximate was not found.

  Make sure the source file exists, and that it has been provided as an input to the compiler.

I see that this module is from sharkdp/purescript-numbers; I'm sorry to have another request, but as a general rule the purescript-contrib libraries only have dependencies on either purescript libraries or other purescript-contrib libraries.

That's because when there's a breaking change to the ecosystem (for example, the new 0.14 compiler release) we need to make sure we can apply the updates to all PureScript and PureScript Contrib libraries without being blocked by maintainers that might be out of town, on vacation, unresponsive, etc.

Are you able to either a) inline this function into the test source code or b) test this equality some other way, like a regular == or by multiplying to make the results integers and compare those instead? Just something that won't incur the dependency on a non-contrib library.

@thomashoneyman thomashoneyman self-assigned this Apr 8, 2020
@thomashoneyman thomashoneyman merged commit f0179b0 into purescript-contrib:master Apr 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants