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

Hide alt values #136

Closed
wants to merge 2 commits into from
Closed

Hide alt values #136

wants to merge 2 commits into from

Conversation

movermeyer
Copy link
Collaborator

What are you trying to accomplish?

Values in CLDR can have an alt attribute. While we figure out how we want to export them, we should ignore them so they don't accidentally get output.

Ref #125.

What approach did you choose and why?

ruby-cldr had a alt? method that was being used in a few places to ignore these values, but it wasn't being consistently.

I moved the filtering into DataFile, so the rest of the code doesn't even get a chance to see these values, so they can't possibly forget to remove them (thereby leaking them).

What should reviewers focus on?

🤷

The impact of these changes

alt values are hidden and not exported.

Testing

...

Checklist

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

Until we know what we want to do with them. Ref #125
These values are being removed by `DataFile`
@movermeyer
Copy link
Collaborator Author

movermeyer commented Apr 21, 2022

This is not quite right. ruby-cldr already uses alt values in some cases (e.g., narrow-symbol keys in Currencies component).

Perhaps we'll do an allowlist of where we expect alt attributes would be better to know that we are filtering them out appropriately.

@movermeyer movermeyer closed this Apr 21, 2022
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