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

Validateur NeTEx : validation "à la demande" #4158

Merged
merged 27 commits into from
Sep 17, 2024

Conversation

ptitfred
Copy link
Contributor

@ptitfred ptitfred commented Sep 3, 2024

Cette PR introduit le support du format NeTEx dans la validation à la demande en ajustant le job Transport.Jobs.OnDemandValidationJob et le controlleur et templates associés du validateur à la demande.

Des aperçus du résultat de la validation :

image

image

Les résultats de validation ne sont pas encore en effet très utile au grand public.


Voir #4153.

@ptitfred ptitfred force-pushed the netex-validator/on-demand-live branch 7 times, most recently from 47e19b1 to dadbc59 Compare September 12, 2024 10:15
@ptitfred ptitfred force-pushed the netex-validator/on-demand-live branch from dadbc59 to 8403b88 Compare September 12, 2024 10:27
@ptitfred
Copy link
Contributor Author

Après plusieurs passages, je suis arrivé à la conclusion qu'extraire dans d'autres PRs plus petites certains de ces changements serait trop artificiel.

Je la sors donc de draft pour qu'on puisse la review.

@ptitfred ptitfred marked this pull request as ready for review September 12, 2024 10:33
@ptitfred ptitfred requested a review from a team as a code owner September 12, 2024 10:33
@ptitfred ptitfred mentioned this pull request Sep 12, 2024
15 tasks
@AntoineAugusti AntoineAugusti self-assigned this Sep 12, 2024
Copy link
Member

@AntoineAugusti AntoineAugusti left a comment

Choose a reason for hiding this comment

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

Très bon boulot @ptitfred, félicitations 👏 Bravo d'avoir réussi à comprendre et modifier la validation à la demande et les différents templates. La mise en commun GTFS / NeTEx est à saluer !

Je t'ai fait quelques suggestions/point d'attention. Ce sera bon bientôt à envoyer en production.

@@ -104,6 +104,71 @@ defmodule Shared.DateTimeDisplay do

def format_datetime_to_paris(nil, _, _), do: ""

@doc """
Formats a duration in seconds to display, according to a locale.
Copy link
Member

Choose a reason for hiding this comment

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

Si tu souhaites, on a déjà :cldr_utils en dépendance et il devrait y avoir moyen d'utiliser https://hexdocs.pm/ex_cldr_calendars/Cldr.Calendar.Duration.html

Il faudrait mettre à jour la liste de providers

use Cldr, locales: ["en", "fr"], providers: [Cldr.Number], default_locale: "fr"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah merci, ça me fendait le coeur de le faire moi-même...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fait.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

J'ai dû exclure lib/cldr.ex de dialyzer faute d'avoir une bonne correction. L'erreur :

lib/cldr.ex:1:overlapping_contract
Overloaded contract for Transport.Cldr.Calendar.localize/3 has
overlapping domains; such contracts are currently unsupported and
are simply ignored.

Copy link
Member

Choose a reason for hiding this comment

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

Tu peux ouvrir une issue là-bas ? J'étais tombé sur un bug similaire récemment elixir-cldr/cldr#236

Copy link
Contributor Author

Choose a reason for hiding this comment

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

apps/transport/priv/gettext/fr/LC_MESSAGES/validations.po Outdated Show resolved Hide resolved
@ptitfred
Copy link
Contributor Author

Dialyzer is mean to me.

@ptitfred
Copy link
Contributor Author

Très bon boulot @ptitfred, félicitations 👏 Bravo d'avoir réussi à comprendre et modifier la validation à la demande et les différents templates. La mise en commun GTFS / NeTEx est à saluer !

Je t'ai fait quelques suggestions/point d'attention. Ce sera bon bientôt à envoyer en production.

Merci ; je pense que c'est à jour en réaction à tes commentaires.

Copy link
Member

@AntoineAugusti AntoineAugusti left a comment

Choose a reason for hiding this comment

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

Merci pour la prise en compte de mes suggestions et les améliorations apportées 👏

@ptitfred ptitfred added this pull request to the merge queue Sep 17, 2024
Merged via the queue into master with commit 2fc9d35 Sep 17, 2024
4 checks passed
@ptitfred ptitfred deleted the netex-validator/on-demand-live branch September 17, 2024 15:34
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.

2 participants