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

Fix list parameter parsing #267

Merged
merged 1 commit into from
Nov 30, 2024
Merged

Fix list parameter parsing #267

merged 1 commit into from
Nov 30, 2024

Conversation

movermeyer
Copy link
Collaborator

@movermeyer movermeyer commented Nov 30, 2024

What are you trying to accomplish?

✗ thor cldr:export --components Plurals --target=./data
Expected all values of '--components' to be one of Aliases, Calendars, Characters, ContextTransforms, CountryCodes, Currencies, 
CurrencyDigitsAndRounding, Delimiters, Fields, Languages, Layout, LikelySubtags, 
Lists, Metazones, NumberingSystems, Numbers, ParentLocales, PluralRules, Plurals, 
Rbnf, RbnfRoot, RegionCurrencies, RegionValidity, SegmentsRoot, Subdivisions, 
Territories, TerritoriesContainment, Timezones, Transforms, Units, Variables, 
WeekData, WindowsZones; got Plurals

Broke in thor v1.3.0, when the values of enum began being validated.
enum is expected to be Strings, but we were passing in Symbols.

rails/thor#784

What approach did you choose and why?

  1. Fixed the Symbol/String confusion.
  2. Removed the extra validation we were doing
  3. Bumped the minimum version of Thor to 1.3.0.
    • So people aren't using an older version without the validation

What should reviewers focus on?

🤷

The impact of these changes

These list/enum parameters will work again

Testing

✗ thor cldr:export --components Plurals --target=./data

Checklist

  • I have added a CHANGELOG entry for this change (or determined that it isn't needed)

@movermeyer movermeyer marked this pull request as ready for review November 30, 2024 23:04
@movermeyer movermeyer merged commit e97c662 into main Nov 30, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant