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 TOML encoding #712

Merged
merged 2 commits into from
Oct 8, 2024
Merged

Add TOML encoding #712

merged 2 commits into from
Oct 8, 2024

Conversation

vincent-botbol
Copy link
Contributor

@vincent-botbol vincent-botbol commented Oct 3, 2024

This PR cleans the clerk's TOML config ad hoc reader/writer by adding a generic type-safe encoder/decoder.
This allows to properly describe and update config files without much boilerplate. C.f. the second commit that replaces the existing parsing with this encoder.

The PR adds new fields to the config file (related to #692) that are currently unused but are retro-compatible with the existing TOML configuration files (in particular, the one present in https://github.com/CatalaLang/catala-examples).

It also displays proper error messages, for example:
image
or
image

@vincent-botbol vincent-botbol force-pushed the vbot@clerk-toml-encoding branch from 9aa289b to d86de8f Compare October 3, 2024 13:26
@denismerigoux denismerigoux added ✨ enhancement New feature or request 🏗️ build system Build system or Makefile labels Oct 3, 2024
Copy link
Contributor

@denismerigoux denismerigoux left a comment

Choose a reason for hiding this comment

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

This is an excellent PR! I <3 the error messages. The code in clerk_config is maybe too type-safe with heavy use of combinators that might be hard to read for OCaml newbies, but let's assume clerk will be maintained by seasoned OCaml devs :)

@@ -0,0 +1,536 @@
(* This file is part of the Catala build system, a specification language for
tax and social benefits computation rules. Copyright (C) 2024 Inria,
contributors: Louis Gesbert <[email protected]>
Copy link
Contributor

Choose a reason for hiding this comment

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

Wrong ;)

Copy link
Contributor

@AltGr AltGr left a comment

Choose a reason for hiding this comment

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

This very nice!

@AltGr AltGr merged commit d7caebb into master Oct 8, 2024
5 checks passed
@AltGr AltGr deleted the vbot@clerk-toml-encoding branch October 8, 2024 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏗️ build system Build system or Makefile ✨ enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants