Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Upgrades to newer version of intl #3

Merged
merged 1 commit into from
Sep 6, 2023
Merged

Conversation

mwcbrent
Copy link
Contributor

@mwcbrent mwcbrent commented Sep 5, 2023

Caveat: I'm not really much of a dart / flutter engineer so apologies if I broke or removed something that shouldn't be.

This pull request does the following:

  • Bumps intl version to 18
  • Removes pedantic as it is now deprecated (not sure what to replace this with)
  • Adds a helper to the tests. The matchers were getting hung up on a NNBSP character being returned.

Wanted to share if this was at all helpful to anybody else.

@rknell
Copy link
Owner

rknell commented Sep 6, 2023

Thanks @mwcbrent

I was going to ask why you did what you did but I pulled the code down and checked it, then went down a rabbit hole to find why it broke.

I disagree with the philosophy of the intl guys and changing the whitespace should be a breaking change - as because - it broke our tests and also other peoples code as referenced here: dart-lang/i18n#637. So what I'm going to do is pull down your changes (thank you!) but revert to using the hard coded strings as their attitude towards breaking code worries me and I would like to know about changes in their API going forward even if its just whitespace.

@rknell rknell merged commit dae4f89 into rknell:master Sep 6, 2023
@rknell
Copy link
Owner

rknell commented Sep 6, 2023

Hey @mwcbrent - I also want to add though, I really appreciate the way you went about making the pull request and not just changing the text in the test to "make it work". Your dart engineering is great.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants