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

✨ Add types for Categories and Subcategories #2458

Merged
merged 3 commits into from
Oct 14, 2024
Merged

✨ Add types for Categories and Subcategories #2458

merged 3 commits into from
Oct 14, 2024

Conversation

bjlaa
Copy link
Contributor

@bjlaa bjlaa commented Oct 1, 2024

No description provided.

@bjlaa bjlaa requested a review from paulsouche October 1, 2024 15:22
@bjlaa bjlaa self-assigned this Oct 1, 2024
Copy link

Copy link

github-actions bot commented Oct 1, 2024

Report for the pull request #2458

🚀 Test the model from the website: https://preprod.nosgestesclimat.fr?PR=2458

🌐 Translation status

You will find more information about the translation in our wiki.

Rules

Language Nb. missing translations Status
en Ø ✔️
es Ø ✔️

Personas

Language Nb. missing translations Status
en Ø ✔️
es Ø ✔️

👫 Personas changes

Test personas bilans against preprod

In details

Marie

Règle PR nightly Δ (%)
Aucune différence détectée sur 2329 tests

Yoram

Règle PR nightly Δ (%)
Aucune différence détectée sur 2329 tests

Corentin

Règle PR nightly Δ (%)
Aucune différence détectée sur 2329 tests

Sandy

Règle PR nightly Δ (%)
Aucune différence détectée sur 2329 tests

Mehdi

Règle PR nightly Δ (%)
Aucune différence détectée sur 2329 tests

Sylviane

Règle PR nightly Δ (%)
Aucune différence détectée sur 2329 tests

Jessica

Règle PR nightly Δ (%)
Aucune différence détectée sur 2329 tests

Nolan

Règle PR nightly Δ (%)
Aucune différence détectée sur 2329 tests

Anne Claire

Règle PR nightly Δ (%)
Aucune différence détectée sur 2329 tests

Gérard

Règle PR nightly Δ (%)
Aucune différence détectée sur 2329 tests

Test personas bilans against production

Show

Marie

Règle PR latest Δ (%)
Aucune différence détectée sur 2329 tests

Yoram

Règle PR latest Δ (%)
Aucune différence détectée sur 2329 tests

Corentin

Règle PR latest Δ (%)
Aucune différence détectée sur 2329 tests

Sandy

Règle PR latest Δ (%)
Aucune différence détectée sur 2329 tests

Mehdi

Règle PR latest Δ (%)
Aucune différence détectée sur 2329 tests

Sylviane

Règle PR latest Δ (%)
Aucune différence détectée sur 2329 tests

Jessica

Règle PR latest Δ (%)
Aucune différence détectée sur 2329 tests

Nolan

Règle PR latest Δ (%)
Aucune différence détectée sur 2329 tests

Anne Claire

Règle PR latest Δ (%)
Aucune différence détectée sur 2329 tests

Gérard

Règle PR latest Δ (%)
Aucune différence détectée sur 2329 tests

prepack.mjs Outdated
'services sociétaux'
]
function generateCategoriesTypes(destPath) {
const rawData = fs.readFileSync(destPath)

Choose a reason for hiding this comment

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

Ca sert pas rawData si ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tout à fait, ce qui me fait remarquer que nous n'avons pas de linter côté modèle pour les fichiers JS ?

fs.writeFileSync(
'./types/subcategories.d.ts',
generateSubcategoriesTypes(destPath)
)

Choose a reason for hiding this comment

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

Sync, sync, sync everywhere ! We should have a main async function and use sf promises

Copy link
Contributor

Choose a reason for hiding this comment

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

Salut @paulsouche, j'ai pas mal contribué sur cette base de code et en particulier sur les scripts de compilation du modèle.

Je ne suis pas très calé en JS et me demandais pourquoi il serait préférable d'utiliser des promises ici ? Le code étant séquentiel et les fichiers petits, je ne vois pas trop ce que ça apporterait 🤔

Choose a reason for hiding this comment

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

Hello ! Tu peux faire plusieurs opérations en parallèle au lieu de bloquer le thread

Copy link
Contributor

Choose a reason for hiding this comment

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

Dans ce cas précis, j'ai peur que ça complexifie le code pour pas de réels gains de performances.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pas d'opinion forte ici, je propose de merger cette PR qui n'a pas trait au refactoring de ce script 😄

@bjlaa bjlaa merged commit d9359f1 into preprod Oct 14, 2024
7 checks passed
bjlaa added a commit that referenced this pull request Oct 17, 2024
* Modifs linge + cuisine

* Suppression division par foyer

* Voiture propriétaire par défaut

* Hausse lessives par habitant

* Fix par défaut voiture

* par défaut voiture modif

* Plus d'eau jardin

* Hausse cuisine + vaisselle

* Encore plus de lessives

* Ajout lien ressource ADEME eau domestique

* Traductions

* Ajout de notes

* Français devient humain

* Traductions

* Fix trads

* ✨ Add types for Categories and Subcategories (#2458)

* ✨ Add types for Categories and Subcategories

* ♻️ Remove unused var

* Ch model adaptations (#2454)

* Modifications CH-fr model June 2024

* Add notes and small modification CH-fr July 2024

* Modification August on Actions/agir

* Fix CH-fr model to renamed base rules keys + translate CH-es and CH-en using DeepL

* Minor Fix requested by NGC.fr team

---------

Co-authored-by: jrichard <[email protected]>
Co-authored-by: Benjamin Arias <[email protected]>

* 🔖 Release 3.3.0

* 🗑️ Remove unused translations

---------

Co-authored-by: JuliePouliquen <[email protected]>
Co-authored-by: Cedric Gampert <[email protected]>
Co-authored-by: jrichard <[email protected]>
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.

3 participants