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

Remove the F and FT alias (pending auto-migration tool) #79

Open
srghma opened this issue Jul 10, 2020 · 6 comments
Open

Remove the F and FT alias (pending auto-migration tool) #79

srghma opened this issue Jul 10, 2020 · 6 comments
Labels
status: blocked This issue or PR is blocked by something and cannot make progress. type: breaking change A change that requires a major version bump.

Comments

@srghma
Copy link

srghma commented Jul 10, 2020

it's useless alias that makes code harder to read (not my words, but I agree)

type F = Except MultipleErrors

but this will be a breaking change

@kl0tl
Copy link
Member

kl0tl commented Dec 21, 2020

I would be in favor of dropping the alias. It allows to document the behaviour of the Alt instance for Except though (<|> accumulates foreign errors instead of keeping only the first one as it does for Either), so perhaps we should rather rename it to something more meaningful?

What we decide here will have consequences for #74.

@hdgarrood
Copy link

I'm not in favour of this. I generally agree that writing out Except MultipleErrors is better, but imo it's not better enough that we can justify asking people to rewrite existing code to silence deprecation warnings. I think we should wait until we have an automatic upgrading tool before tackling this.

@JordanMartinez JordanMartinez added purs-0.15 A reminder to address this issue or merge this PR before we release PureScript v0.15.0 type: breaking change A change that requires a major version bump. labels Dec 4, 2021
@JordanMartinez
Copy link
Contributor

I'm in favor of dropping the alias. I'm not sure when we'll get a automatic upgrading tool.

srghma added a commit to srghma/purescript-web3-generator that referenced this issue Feb 18, 2022
@JordanMartinez
Copy link
Contributor

How about we update all code in this library to stop using the F and FT aliases, but otherwise keep them until we get a auto-migrate tool? For example

-- before
type FT = ExceptT MultipleErrors
unsafeReadTagged :: forall m a. Monad m => String -> Foreign -> FT m a
unsafeReadTagged tag value = ...

-- after
type FT = ExceptT MultipleErrors
unsafeReadTagged :: forall m a. Monad m => String -> Foreign -> ExceptT (NonEmptyList ForeignError) m a
unsafeReadTagged tag value = ...

@garyb
Copy link
Member

garyb commented Mar 15, 2022

Sounds like a good compromise to me 👍

@JordanMartinez
Copy link
Contributor

Done!

@JordanMartinez JordanMartinez changed the title remove the F alias Remove the F and FT alias (pending auto-migration tool) Mar 15, 2022
@JordanMartinez JordanMartinez removed the purs-0.15 A reminder to address this issue or merge this PR before we release PureScript v0.15.0 label Mar 15, 2022
@JordanMartinez JordanMartinez added the status: blocked This issue or PR is blocked by something and cannot make progress. label Mar 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: blocked This issue or PR is blocked by something and cannot make progress. type: breaking change A change that requires a major version bump.
Projects
None yet
Development

No branches or pull requests

5 participants