refactor(l10n): rename weekday_year_* to weekday_cycle_*

[tukusejssirs]

No description provided.

[tukusejssirs]

@emagnier, one test fails when using node@19.0.1 and npm@8.19.2 unless various Jest-related packages are updated, which you already took care of in #404, thus I suggest to merge this after that PR.

I also considered renaming sunday_year_* to use Sunday cycle (as it is used in the English translation of the Roman Missal), however sunday_cycle_year_* sounds like too much, as individual Sunday cycles are usually called Year *. It might make sense (from logical point of view) to change cycles structure in Locale interface to something like below, however, from KISS point of view it might be too nested without any gain. We might consider this in #365 though.

cycle = {
   proper?: {
      time?: string,
      time?: string,
   },
   synday_cycle?: {
      year_a?: string,
      year_b?: string,
      year_c?: string
   },
   weekday_cycle?: {
      cycle_1?: string,
      cycle_2?: string
   },
   psalter_cycle: {
      week_1?: string,
      week_2?: string,
      week_3?: string,
      week_4?: string
   }
}

[emagnier]

Just renaming these keys are not enough. You also need to modify the corresponding constants here:

romcal/lib/constants/cycles.ts

Lines 58 to 61 in ec5b030

	export const WeekdayCycles = {
	Year1: 'YEAR_1',
	Year2: 'YEAR_2',
	} as const;

You should try to run romcal locally, and check the computed localized labels. FYI, snapshots tests added here #404 are testing computed data without localized metadata (this is why tests are keeping running happily, but this might be something to note, to add coverage on this topic here).

[tukusejssirs]

@emagnier, sorry for late reply, I’ve been busy at work. 😉

Just renaming these keys are not enough. You also need to modify the corresponding constants here:

romcal/lib/constants/cycles.ts

Lines 58 to 61 in ec5b030

	export const WeekdayCycles = {
	Year1: 'YEAR_1',
	Year2: 'YEAR_2',
	} as const;

I was a bit of a mental exercise to me in order to understand why that is required. I understand that there needs to be a place where the weekday localisation is done, however, I wanted to understand why it is needed to change it there. Now I understand why it is required to do there! 🙏

FYI, snapshots tests added here #404 are testing computed data without localized metadata (this is why tests are keeping running happily, but this might be something to note, to add coverage on this topic here).

I haven’t actually tested it with snapshots yet. 😉 Now I have fixed that and updated the snapshots. 😉

[emagnier]

On a second thought, I'm ultimately not too sure about this modification.

I find it good to keep the reference of the year in this property name (the time unit of this cycle). Like sunday_year_a or psalter_week_1. What do you think?

[tukusejssirs]

I find it good to keep the reference of the year in this property name (the time unit of this cycle). Like sunday_year_a or psalter_week_1. What do you think?

Well, I get you say. Your idea it to keep the unit in the names (in the codebase only or also in the localised strings?), my idea is to keep the name (and usage) in sync with the official name if possible. Now, I re-searched what is the current name of the cycles.

According to USCCB, the word cycle is used for both Sunday and weekday cycles, but (as I now understand it) the cycle is a multi-year period, therefore a Sunday cycle is a three-year period and a weekday cycle is a two-year period. Then, each year in a cycle has its name: Year A-C in Sunday cycle and Year I-II in weekday cycle.

Based on this logic, whenever we talk about the whole multi-year period, we should use the term cycle; when we talk about a year of that period, then we should use the term year. Therefore cycle object in the romcal output object should have properties ending with Cycle (i.e. properCycle, sundayCycle, weekdayCycle), but their values should be YEAR_* (e.g. YEAR_A, YEAR_II).

And this is the case already, although we use YEAR_1 and YEAR_2 instead of YEAR_I and YEAR_II. Would you accept this small change (using Roman Numerals)?

---

Anyway, I have no idea how to get the localised cycle name (any cycle, like Sunday/weekday/psalter). How can I get it? 🙏

[emagnier]

Thank you for this last message and sharing your thought.

keep the unit in the names (in the codebase only or also in the localised strings?)

-> In the codebase only. The localized strings should reflect the language usages, as it is now.

I'm not against YEAR_I and YEAR_II. But we should do the same with WEEK_I, WEEK_II...
As long as romcal is not final released yet (v3), this is still the good time to do these fine-tuning.

[tukusejssirs]

-> In the codebase only. The localized strings should reflect the language usages, as it is now.

Agreed.

I'm not against YEAR_I and YEAR_II. But we should do the same with WEEK_I, WEEK_II... As long as romcal is not final released yet (v3), this is still the good time to do these fine-tuning.

Agreed. Therefore, I’d like this to be changed before stable v3 release. I still want to finish #393, but I’ve got blocked by different things (work, SSD drive failure, consolidation of multiple celebration sources).