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

feat: sector I and III #623

Merged
merged 2 commits into from
Aug 6, 2024
Merged

feat: sector I and III #623

merged 2 commits into from
Aug 6, 2024

Conversation

mfonsecaOEF
Copy link
Contributor

No description provided.

@mfonsecaOEF mfonsecaOEF requested a review from evanp August 6, 2024 02:21
@mfonsecaOEF mfonsecaOEF self-assigned this Aug 6, 2024
Copy link
Contributor

@evanp evanp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please install prettier and use it in your code editor.

Generally, looks good. Thanks for this hard work!

@evanp
Copy link
Contributor

evanp commented Aug 6, 2024

@mfonsecaOEF this won't merge because you changed a bunch of unique IDs for direct measure to just "direct measure".

That was a mistake. We need unique IDs for Direct Measure for each and every sector because the labels are different: "Direct measure for rail transport", etc.

@thehighestprimenumber
Copy link
Contributor

I'm creating the direct measure myself in the code, so you don't need to worry about the ids.

@evanp
Copy link
Contributor

evanp commented Aug 6, 2024

@thehighestprimenumber I don't believe you can implement "Direct measure for X" in a language-independent way.

@evanp evanp merged commit 5e1b080 into develop Aug 6, 2024
2 of 3 checks passed
@evanp evanp deleted the MI_sectors_I_and_III branch August 6, 2024 13:33
@thehighestprimenumber
Copy link
Contributor

@thehighestprimenumber I don't believe you can implement "Direct measure for X" in a language-independent way.
@evanp
I already am doing it, since I didn't have the strings in the json I'm building them myself and they are already being translated as we saw today

@evanp
Copy link
Contributor

evanp commented Aug 8, 2024

Awesome. I should have been clearer; I don't think it's a good idea to do something like this:

const dm = getString('direct-measure-for');
const t = getString(topicId);

const label = dm + ' ' + t;

Assembling strings like this would work for en, es, and maybe pt, but not other languages.

I think you are (were) just building the ID, not trying to build the string from parts. So that's fine!

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.

None yet

3 participants