Skip to content

Conversation

@milesfrain
Copy link
Member

Description of the change

Fixes spago warnings when building and running tests for this library locally.


Checklist:

  • Added the change to the changelog's "Unreleased" section with a link to this PR and your username

@milesfrain
Copy link
Member Author

Having trouble reproducing the CI failure locally. This command works fine for me:

spago build --purs-args '--censor-lib --strict --censor-codes="UserDefinedWarning"'
purs --version
0.14.1
spago --version
0.20.1

@thomashoneyman
Copy link
Contributor

thomashoneyman commented May 10, 2021

We can remove the unused function — that’s a 0.14.1 warning. I’m not sure why you aren’t seeing it locally.

Comment on lines +45 to 53
{-
let
oneIfA = 1 <$ string "a" <?> "Letter was 'a'"
zeroIfNotA = 0 <$ regex "[^a]" <?> "Letter was not 'a'"
letterIsOneOrZero = oneIfA <|> zeroIfNotA <?>
"The impossible happened: \
\a letter was not 'a', and was not not-'a'."
convertLettersToList = many1 letterIsOneOrZero
{-
list <- convertLettersToList -}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't know if we should fully remove unused code found here and in other locations in this file

Copy link
Contributor

@thomashoneyman thomashoneyman May 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be removed if it's unused (unless specifically commented out with a TODO or something).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point of this comment was to clarify what the below code is doing in a more piece-meal way.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jordan, do you prefer it remains commented out or removed? Whatever your choice we can then merge and release.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer it stay commented out.

@thomashoneyman thomashoneyman merged commit a0bebf1 into purescript-contrib:main May 11, 2021
@milesfrain milesfrain deleted the fix-spago-dep branch May 12, 2021 02:16
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.

3 participants