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

Tech: Ajouter des méthodes pour tracer les types d'enterprise SIAEs #5535

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

calummackervoy
Copy link
Contributor

🤔 Pourquoi ?

#5498 (comment)

Propostion de refacteur pour améliorer la lisibilité, inclure un test qui protège l'ajout des nouveaux valeurs sur CompanyKind

🚨 À vérifier

  • Mettre à jour le CHANGELOG_breaking_changes.md ?
  • Ajouter l'étiquette « Bug » ?

Improve readability. Also includes a test designed to require any new company kinds are explicitly defined as SIAE or not, to prevent bugs
# SIAEs that have a convention.
# Ported older comment: ASP data is used to keep the siae data of these kinds in sync.
# These kinds and only these kinds thus have convention/AF logic.
SIAE_WITH_CONVENTION_KINDS = [
Copy link
Contributor

Choose a reason for hiding this comment

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

A mon avis, un simple renommage SIAE_WITH_CONVENTION_KINDS -> SIAE_KINDS pourrait suffire.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Donc pas trop le choix je crois 😅.
SIAE_KINDS fonctionne bien pour l'IAE mais pour les EA/EATT ça fait un truc bizarre (EA_EATT_KINDS) je me demande si faudrait pas plutôt utiliser un truc plus long, genre COMPANY_WITH_IAE_CONVENTION et ainsi pouvoir faire COMPANY_WITH_${BIDULE ADMINISTRATIF} mais bon c'est pas non plus critique et on pourra toujours renommer plus tard :).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@xavfernandez suffisant oui mais je trouve la méthode plus lisible ? En particulier si on prévoit inclure des autres méthodes pour les autres groupages ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants