Several minor fixes #88

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
@yarons

yarons commented Dec 13, 2012

ה shouldn't have geresh after it, otherwise it's God's name.
שבת is not a day, שבת is a queen not a day thus cannot be called יום שבת.

Several minor fixes
ה shouldn't have geresh after it, otherwise it's God's name.
שבת is not a day, שבת is a queen not a day thus cannot be called יום שבת.
@KL-7

This comment has been minimized.

Show comment
Hide comment
@KL-7

KL-7 Dec 13, 2012

Contributor

I'm not sure how @camertron will decide to handle fixes like this, but the thing is, that most of the TwitterCLDR's YAML resources are generated from the CLDR data (currently CLDR v21.0 is used). For example, calendars data for Hebrew was imported from here.

The problem is that if this PR is merged, all your fixes might disappear next time someone regenerates YAML resources from the CLDR data. One solution for that is to submit your fixes directly to CLDR. Another is to develop some mechanism for handling local TwitterCLDR fixes that will override the data imported from CLDR, but it doesn't sound like a good idea to me.

Contributor

KL-7 commented Dec 13, 2012

I'm not sure how @camertron will decide to handle fixes like this, but the thing is, that most of the TwitterCLDR's YAML resources are generated from the CLDR data (currently CLDR v21.0 is used). For example, calendars data for Hebrew was imported from here.

The problem is that if this PR is merged, all your fixes might disappear next time someone regenerates YAML resources from the CLDR data. One solution for that is to submit your fixes directly to CLDR. Another is to develop some mechanism for handling local TwitterCLDR fixes that will override the data imported from CLDR, but it doesn't sound like a good idea to me.

@yarons

This comment has been minimized.

Show comment
Hide comment
@yarons

yarons Dec 13, 2012

These patches were coordinated with @camertron before hand, thank you very much!

yarons commented Dec 13, 2012

These patches were coordinated with @camertron before hand, thank you very much!

@camertron

This comment has been minimized.

Show comment
Hide comment
@camertron

camertron Dec 13, 2012

Collaborator

Hey @yarons,

Kirill (@KL-7), who wrote quite a bit of TwitterCLDR's code, is absolutely right. When we conversed via email, I neglected to mention you'd need to make any locale-specific changes in the resources/custom/locales directory, which overrides the generated yaml files in resources/locales. It should be easy to move your changes over. For example, resources/custom/locales/he/calendars.yml would look like this (just a few of your changes shown):

:calendars:
  :gregorian:
    :additional_formats:
      :Ed: E ה־d
    :days:
      :format:
        :abbreviated:
          :thu: יום ה
        :narrow:
          :sun: א׳
          :thu: ה
          :tue: ג׳
          :wed: ד׳
          :mon: ב׳

Does that make sense?

Collaborator

camertron commented Dec 13, 2012

Hey @yarons,

Kirill (@KL-7), who wrote quite a bit of TwitterCLDR's code, is absolutely right. When we conversed via email, I neglected to mention you'd need to make any locale-specific changes in the resources/custom/locales directory, which overrides the generated yaml files in resources/locales. It should be easy to move your changes over. For example, resources/custom/locales/he/calendars.yml would look like this (just a few of your changes shown):

:calendars:
  :gregorian:
    :additional_formats:
      :Ed: E ה־d
    :days:
      :format:
        :abbreviated:
          :thu: יום ה
        :narrow:
          :sun: א׳
          :thu: ה
          :tue: ג׳
          :wed: ד׳
          :mon: ב׳

Does that make sense?

@KL-7

This comment has been minimized.

Show comment
Hide comment
@KL-7

KL-7 Dec 13, 2012

Contributor

So, TwitterCLDR already has a way to patch CLDR data. Pretty cool!

Though, if these are definite errors in the CLDR data, I think it'd be nice to report them there as well. It might take some time, but in the end much more people will benefit from these fixes. Probably @srl295 could provide some guidance on that matter.

Contributor

KL-7 commented Dec 13, 2012

So, TwitterCLDR already has a way to patch CLDR data. Pretty cool!

Though, if these are definite errors in the CLDR data, I think it'd be nice to report them there as well. It might take some time, but in the end much more people will benefit from these fixes. Probably @srl295 could provide some guidance on that matter.

@srl295

This comment has been minimized.

Show comment
Hide comment
@srl295

srl295 Dec 13, 2012

@KL-7 @camertron did you get to look at the new CLDR JSON format? (see cldr-users list or cldr homepage) - please give feedback there.

Absolutely- if there's an issue, I'd really recommend filing a bug in cldr FIRST and simply referencing it here. In ICU for example there used to be a message saying, do NOT even file data bugs here- just in CLDR.

Do you really want to manage local patches? probably not. Let's keep the C in CLDR for Common.

srl295 commented Dec 13, 2012

@KL-7 @camertron did you get to look at the new CLDR JSON format? (see cldr-users list or cldr homepage) - please give feedback there.

Absolutely- if there's an issue, I'd really recommend filing a bug in cldr FIRST and simply referencing it here. In ICU for example there used to be a message saying, do NOT even file data bugs here- just in CLDR.

Do you really want to manage local patches? probably not. Let's keep the C in CLDR for Common.

@camertron

This comment has been minimized.

Show comment
Hide comment
@camertron

camertron Dec 13, 2012

Collaborator

@srl295 Yes, I did get a chance to look at the new JSON format, it looks fine. Apologies for not giving feedback :( The JSON is very similar to the yaml we have generated with the ruby-cldr gem, so it should be pretty straightforward to integrate. To be honest though, we could work with just about any format - we just need something to parse/use. The JSON is much more convenient because very little data massaging will be necessary, unlike the current XML format. Any idea if/when the JSON will go live? Where should I give feedback?

@yarons, would you mind filing a bug with CLDR Trac? We'd love to have this fixed in the Common repo of course. @srl295 naturally we don't want to manage local patches, but occasionally it's necessary for Twitter-specific content. I don't think we've actually modified CLDR data until now - it's all been additive so far.

Collaborator

camertron commented Dec 13, 2012

@srl295 Yes, I did get a chance to look at the new JSON format, it looks fine. Apologies for not giving feedback :( The JSON is very similar to the yaml we have generated with the ruby-cldr gem, so it should be pretty straightforward to integrate. To be honest though, we could work with just about any format - we just need something to parse/use. The JSON is much more convenient because very little data massaging will be necessary, unlike the current XML format. Any idea if/when the JSON will go live? Where should I give feedback?

@yarons, would you mind filing a bug with CLDR Trac? We'd love to have this fixed in the Common repo of course. @srl295 naturally we don't want to manage local patches, but occasionally it's necessary for Twitter-specific content. I don't think we've actually modified CLDR data until now - it's all been additive so far.

@srl295

This comment has been minimized.

Show comment
Hide comment
@srl295

srl295 Dec 17, 2012

You could file a CLDR bug in response to the format, or the cldr-users list
http://www.unicode.org/consortium/distlist.html#cldr_list

I'd recommend joining cldr-users

On Thu, Dec 13, 2012 at 4:02 PM, Cameron Dutro notifications@github.comwrote:

@srl295 https://github.com/srl295 Yes, I did get a chance to look at
the new JSON format, it looks fine. Apologies for not giving feedback :(
The JSON is very similar to the yaml we have generated with the ruby-cldr
gem, so it should be pretty straightforward to integrate. To be honest
though, we could work with just about any format - we just need something
to parse/use. The JSON is much more convenient because very little data
massaging will be necessary, unlike the current XML format. Any idea
if/when the JSON will go live? Where should I give feedback?

@yarons https://github.com/yarons, would you mind filing a bug with CLDR
Trac http://unicode.org/cldr/trac? We'd love to have this fixed in the *
C*ommon repo of course. @srl295 https://github.com/srl295 naturally we
don't want to manage local patches, but occasionally it's necessary for
Twitter-specific content. I don't think we've actually modified CLDR data
until now - it's all been additive so far.


Reply to this email directly or view it on GitHubhttps://github.com/twitter/twitter-cldr-rb/pull/88#issuecomment-11355559.

srl295 commented Dec 17, 2012

You could file a CLDR bug in response to the format, or the cldr-users list
http://www.unicode.org/consortium/distlist.html#cldr_list

I'd recommend joining cldr-users

On Thu, Dec 13, 2012 at 4:02 PM, Cameron Dutro notifications@github.comwrote:

@srl295 https://github.com/srl295 Yes, I did get a chance to look at
the new JSON format, it looks fine. Apologies for not giving feedback :(
The JSON is very similar to the yaml we have generated with the ruby-cldr
gem, so it should be pretty straightforward to integrate. To be honest
though, we could work with just about any format - we just need something
to parse/use. The JSON is much more convenient because very little data
massaging will be necessary, unlike the current XML format. Any idea
if/when the JSON will go live? Where should I give feedback?

@yarons https://github.com/yarons, would you mind filing a bug with CLDR
Trac http://unicode.org/cldr/trac? We'd love to have this fixed in the *
C*ommon repo of course. @srl295 https://github.com/srl295 naturally we
don't want to manage local patches, but occasionally it's necessary for
Twitter-specific content. I don't think we've actually modified CLDR data
until now - it's all been additive so far.


Reply to this email directly or view it on GitHubhttps://github.com/twitter/twitter-cldr-rb/pull/88#issuecomment-11355559.

@camertron

This comment has been minimized.

Show comment
Hide comment
@camertron

camertron Jan 6, 2014

Collaborator

No activity for about a year, closing.

Collaborator

camertron commented Jan 6, 2014

No activity for about a year, closing.

@camertron camertron closed this Jan 6, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment