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

Migrate configkit #53

Merged
merged 53 commits into from
Oct 15, 2024
Merged

Migrate configkit #53

merged 53 commits into from
Oct 15, 2024

Conversation

jmalloc
Copy link
Member

@jmalloc jmalloc commented Oct 7, 2024

What change does this introduce?

This PR introduces the config package as a replacement for dogmatiq/configkit.

It's not a direct replacement, as the API has changed somewhat, but it is structurally very similar and quite easy to migrate.

Why make this change?

In general it's an effort to reduce the dependency complexity for engines and related packages.

As regards the changes to the API, they are designed to address several shortcomings with configkit:

  • Its inability to represent incomplete and invalid configurations, which prevented us from using the existing static analysis tools in linting / LSP applications
  • Its divergence from the structure of Dogma itself. Over the years Dogma's API has changed, but configkit had not beed updated to match, leading to its inability to represent things like "routing options" and "handler registration options"
  • Its "message role" centric design, which is a concept that is no longer relevant now that Command, Event and Timeout interfaces are distinct.

Is there anything you are unsure about?

No

What issues does this relate to?

#2

@jmalloc jmalloc force-pushed the config branch 8 times, most recently from 3339f80 to b667304 Compare October 14, 2024 05:21
@jmalloc
Copy link
Member Author

jmalloc commented Oct 14, 2024

This is nearing completion and is being used successfully in a branch of testkit. I need to add further tests.

In general, configkit didn't make its way into the public API of the engines, but there are a few exceptions so there will be some minor BC breaks to testkit/engine (which shouldn't affect anyone). I don't think there will be any public-facing changes to verity.

@jmalloc jmalloc marked this pull request as ready for review October 14, 2024 06:08
@jmalloc jmalloc force-pushed the config branch 4 times, most recently from 184932d to 9a7ce13 Compare October 15, 2024 03:19
@jmalloc
Copy link
Member Author

jmalloc commented Oct 15, 2024

I've requested a review, but I don't really expect you to go through this with a fine-toothed comb. Danil, I believe next steps will be for me to start migrating the static analysis code.


// Fidelity describes the configuration's accuracy in comparison to the
// actual configuration that would be used at runtime.
Fidelity Fidelity
Copy link
Member

Choose a reason for hiding this comment

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

I like the term Fidelity for sure 👍

@jmalloc jmalloc merged commit 8c10121 into main Oct 15, 2024
5 checks passed
@jmalloc jmalloc deleted the config branch October 15, 2024 08:37
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