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

Ajout du dispositif Revenu Solidarite Jeune de la métropole de Lyon #189

Merged
merged 9 commits into from
Nov 27, 2023

Conversation

Allan-CodeWorks
Copy link
Contributor

@Allan-CodeWorks Allan-CodeWorks commented Nov 15, 2023

Ticket qui explique les conditions

Added

  • Ajoute le dispositif Revenu Solidarite Jeune de la métropole de Lyon

@Allan-CodeWorks Allan-CodeWorks self-assigned this Nov 15, 2023
@Allan-CodeWorks Allan-CodeWorks requested a review from a team November 15, 2023 11:13
@Allan-CodeWorks Allan-CodeWorks force-pushed the revenu_solidarite_jeune_lyon branch 3 times, most recently from 1d152d2 to dc26a77 Compare November 22, 2023 09:53
Copy link
Contributor

@Shamzic Shamzic left a comment

Choose a reason for hiding this comment

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

Une remarque pour le nom du fichier du rsj, les autres remarques sont des suggestions optionnelles.

Fonctionnel en local !

label = "Éligibilité géographique de la métropole de Lyon"

def formula(menage, period):
return menage("menage_dans_epci_siren_200046977", period)
Copy link
Contributor

Choose a reason for hiding this comment

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

Ca pourrait être bien de tester menage_dans_epci_siren_200046977 un peu à la manière de ce test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Je suis un peu mitigé, le test en question test la epci_test_factory. On peut effectivement lui reprocher de ne vérifier que cela fonctionne que pour une seule EPCI.
Ici on ne fait qu'utiliser cette réforme, tester cette formule revient à tester la réforme epci_test_factory et ce n'est pas vraiment le sujet ici.

Je trouve que la réforme est effectivement un peu légèrement testée mais je n'ai pas pris le temps de voir comment tester mieux et je ne suis pas convaincu par la valeur de dupliquer un test existant en changeant juste la valeur.

Je propose de créer un ticket pour investiguer la question en dehors de cette PR, qu'en penses-tu ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Je propose de créer un ticket pour investiguer la question en dehors de cette PR, qu'en penses-tu ?

Oui bonne idée. J'avais pas compris d'où ça provenait lors de ma review, ça vaudrait effectivement le coup d'ajouter un petit test sur cette méthode dans une autre PR :)

@Allan-CodeWorks Allan-CodeWorks force-pushed the revenu_solidarite_jeune_lyon branch 2 times, most recently from 44aff62 to b62f77e Compare November 22, 2023 16:20
@Allan-CodeWorks Allan-CodeWorks merged commit 06be123 into master Nov 27, 2023
9 checks passed
@Allan-CodeWorks Allan-CodeWorks deleted the revenu_solidarite_jeune_lyon branch November 27, 2023 09:17
@guillett guillett added this to the Passé milestone Mar 22, 2024
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