openfisca / openfisca-france-fiscalite-miniere

French mining tax system for OpenFisca
GNU Affero General Public License v3.0
3 stars 2 forks source link

Corrige la taxe aurifère suite à l'évolution des entités #26

Closed sandcha closed 2 years ago

sandcha commented 2 years ago

Fixes #24

⚠️ La branche de cette PR dépend du fix d'API Web pour permettre son test par calcul d'API.

Ces changements :

Quelques conseils à prendre en compte :

sandcha commented 2 years ago

Attention, tu as ouvert la PR vers fix-taxe-or et pas vers master, c'est normal ?

👍 Pour la branche de référence, c'était fait exprès pour que vous puissiez tester ^^ (on note d'ordinaire un warning dans la description de la PR pour que ce ne soit pas oublié).

Aussi, il faudrait incrémenter la version de OpenFisca-France-Fiscalite-Miniere

Vu que c'est un bugfix, une 3.1.0 semble bien, qu'en penses-tu ? C'est ligne 5 dans le fichier setup.py

Le bump de version est fait post revue pour ne pas avoir à revenir dessus si une autre PR est mergée entre-temps et le changelog reprend l'essentiel du descriptif de la PR. Mais il est vrai que ces conventions sont écrites nulle part. On pourrait les ajouter au CONTRIBUTING peut-être ? 🤔

En l'état actuel de la PR qui ne modifie que la règle de calcul d'une formule, le versionnement serait plutôt en patch selon la règle par défaut qui se base sur le fait que ça n'impacte pas l'interface de réutilisation du modèle.

Ceci dit, si on change la surface_totale de place / d'entité comme dit dans les commentaires plus haut, ce sera non rétro-compatible et donc un bump majeur.

MichaelBitard commented 2 years ago

Attention, tu as ouvert la PR vers fix-taxe-or et pas vers master, c'est normal ?

+1 Pour la branche de référence, c'était fait exprès pour que vous puissiez tester ^^ (on note d'ordinaire un warning dans la description de la PR pour que ce ne soit pas oublié).

Merci pour l'info je ne savais pas

Aussi, il faudrait incrémenter la version de OpenFisca-France-Fiscalite-Miniere Vu que c'est un bugfix, une 3.1.0 semble bien, qu'en penses-tu ? C'est ligne 5 dans le fichier setup.py

Oui 3.1.0 nickel !

Le bump de version est fait post revue pour ne pas avoir à revenir dessus si une autre PR est mergée entre-temps et le changelog reprend l'essentiel du descriptif de la PR. Mais il est vrai que ces conventions sont écrites nulle part. On pourrait les ajouter au CONTRIBUTING peut-être ? thinking

Oui en effet on pourrait faire ça, la chose étrange c'est que la PR ne peut pas passer tant que la version n'a pas été incrémentée, et ça me fait bizarre de relire des PR rouges (mais après, c'est parce que j'ai été habitué comme ça aussi)

En l'état actuel de la PR qui ne modifie que la règle de calcul d'une formule, le versionnement serait plutôt en patch selon la règle par défaut qui se base sur le fait que ça n'impacte pas l'interface de réutilisation du modèle.

Ceci dit, si on change la surface_totale de place / d'entité comme dit dans les commentaires plus haut, ce sera non rétro-compatible et donc un bump majeur.

Bien vu !

sandcha commented 2 years ago

Rebase en cours.