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: revamped taxonomy sort script #9818

Merged
merged 10 commits into from
Mar 7, 2024
Merged

feat: revamped taxonomy sort script #9818

merged 10 commits into from
Mar 7, 2024

Conversation

alexgarel
Copy link
Member

@alexgarel alexgarel commented Feb 20, 2024

to really sort everything (within an entry)

to really sort everything
@github-actions github-actions bot added the 🧬 Taxonomies https://wiki.openfoodfacts.org/Global_taxonomies label Feb 20, 2024
@github-actions github-actions bot added the GitHub Actions Pull requests that update Github_actions code label Feb 26, 2024
@alexgarel alexgarel marked this pull request as ready for review February 26, 2024 16:33
@alexgarel alexgarel requested a review from a team as a code owner February 26, 2024 16:33
Comment on lines +243 to +246
# TODO: add check_taxonomies when taxonomies ready

lint: lint_perltidy
# TODO: add lint_taxonomies when taxonomies ready
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it dependent on this: #9843 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not really, but the other one is now merged.

It's more when taxonomy will be sorted !

return $a_prefix cmp $b_prefix if ($a_prefix ne $b_prefix);
}
return 0 if ($a eq $b);
# en takes precedence over all others
Copy link
Contributor

Choose a reason for hiding this comment

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

One issue is that for some taxonomies, we have chosen to have the xx: or the zz: entry be the canonical name, so that the canonical id does not change if we need to change the main English name. It's the case for nutrients.txt and units.txt

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed, I now never change first entry (for now). I think it's a different decision to change the main language !

@github-actions github-actions bot added the 💥 Merge Conflicts 💥 Merge Conflicts label Mar 6, 2024
@github-actions github-actions bot removed the 💥 Merge Conflicts 💥 Merge Conflicts label Mar 7, 2024
Copy link
Contributor

@stephanegigandet stephanegigandet left a comment

Choose a reason for hiding this comment

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

Looks good, tested on a few files like units and nutrients

@alexgarel alexgarel enabled auto-merge (squash) March 7, 2024 18:24
@alexgarel alexgarel disabled auto-merge March 7, 2024 18:25
erronously removed !
@alexgarel alexgarel enabled auto-merge (squash) March 7, 2024 18:30
Copy link

sonarqubecloud bot commented Mar 7, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@alexgarel alexgarel merged commit fee8d3d into main Mar 7, 2024
12 checks passed
@alexgarel alexgarel deleted the feat-taxonomy-sort branch March 7, 2024 19:00
john-gom pushed a commit that referenced this pull request May 24, 2024
to really sort everything (within an entry)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GitHub Actions Pull requests that update Github_actions code 🧬 Taxonomies https://wiki.openfoodfacts.org/Global_taxonomies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants