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.

Sign up for GitHub

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

Jump to bottom

docs: OpenAPI nutrients enhancements #9513

Merged
merged 3 commits into from
Dec 12, 2023
Merged

docs: OpenAPI nutrients enhancements #9513

merged 3 commits into from
Dec 12, 2023

Conversation

alexgarel
Copy link
Member

No description provided.

@alexgarel alexgarel requested a review from a team as a code owner December 11, 2023 19:29
@github-actions github-actions bot added the 📚 Documentation Documentation issues improve the project for everyone. label Dec 11, 2023
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (97c6593) 49.02% compared to head (c4636d3) 49.02%.
Report is 1 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #9513   +/-   ##
=======================================
  Coverage   49.02%   49.02%           
=======================================
  Files          66       66           
  Lines       20434    20434           
  Branches     4905     4905           
=======================================
  Hits        10018    10018           
  Misses       9147     9147           
  Partials     1269     1269

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

benbenben2
benbenben2 approved these changes Dec 11, 2023
View reviewed changes
Copy link
Collaborator

@benbenben2 benbenben2 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 to me

stephanegigandet
stephanegigandet reviewed Dec 12, 2023
View reviewed changes
docs/api/ref/schemas/product_nutrition.yaml
The nutrition data on the package can be per serving or per 100g.

This is essential to understand if `<nutrient>_value` and `<nutrient>`
values in `nutriments` applies for a serving or for 100g.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add something like: "When writing products, this setting applies to all existing nutrients values for the product, not only the nutrient values sent in the write request. So it should not be changed unless all nutrients values are provided with values that match the nutrition_data_per field."

stephanegigandet
stephanegigandet approved these changes Dec 12, 2023
View reviewed changes
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.

Thank you! I added one suggestion.

@alexgarel alexgarel enabled auto-merge (squash) December 12, 2023 09:33
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Dec 12, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@alexgarel alexgarel merged commit 49f1f5c into main Dec 12, 2023
14 checks passed
@alexgarel alexgarel deleted the docs-openapi-nutrients branch December 12, 2023 10:12
@openfoodfacts-bot openfoodfacts-bot mentioned this pull request Dec 12, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stephanegigandet stephanegigandet stephanegigandet approved these changes

@benbenben2 benbenben2 benbenben2 approved these changes

Assignees

@alexgarel alexgarel

Labels
📚 Documentation Documentation issues improve the project for everyone.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@alexgarel @codecov-commenter @stephanegigandet @benbenben2