search

citizennet / purescript-ocelot

An opinionated component library for Halogen apps
https://citizennet.github.io/purescript-ocelot/
Apache License 2.0
102 stars 15 forks source link

Fix `Ocelot.Data.Currency` functions not working with negative `Cents` values #227

Closed arthurxavierx closed 2 years ago

arthurxavierx commented 2 years ago

What does this pull request do?

The Ocelot.Data.Currency.formatCentsToStrDollars and Ocelot.Data.Currency.parseCentsFromDollarStr did not work with negative Cents values. We fix that by changing their definitions and adding test cases to cover the intended behavior.

Where should the reviewer start?

Commit-by-commit.

How should this be manually tested?

There's no need for manual testing; the automated tests should be enough.

boygao1992 commented 2 years ago

@arthurxavierx Don't forget to bump version like this and make a release.

arthurxavierx commented 2 years ago

@boygao1992 Do you know why the CircleCI build hasn't finished yet?

boygao1992 commented 2 years ago

Hmm I don't see this PR on the job list. image

boygao1992 commented 2 years ago

Weird, I just opened a new branch and the CI is triggered. image

Maybe try pushing to a different branch?

boygao1992 commented 2 years ago

@arthurxavierx I just did that for you creating a new branch arthur/negative-cents-2 and the CI is running. Is it possible to switch this PR to that branch? If not, probably have to open a new PR.

arthurxavierx commented 2 years ago

@boygao1992 Oh it seems like the original branch got built and already merged for some reason 😶 Thank you!

boygao1992 commented 2 years ago

Nvm, since both branches are on the same commit. That CI passing triggered the auto merge.

boygao1992 commented 2 years ago

No problem!