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

Excluding null values from the generated config #1529

Merged
merged 233 commits into from
Jul 6, 2023

Conversation

aaronpowell
Copy link
Contributor

Enabling a setting on the serializer that will exclude null when it's serializing the runtime config file.

Finding missing types
Refactoring some type names
Simplifying the GraphQL naming by doing that when building the config, rather than on demand
Added implementation of JsonConverter.Write methods to generate an initial pass on the config file using CLI
Removed some tests that weren't testing the CLI but dependencies (such as CommandLineParser or ILogger)
…them simpler and clearer with the new object structure
Moved logic for Entities defaults to RuntimeEntities rather than deserialiser, as that is a more logical place. We always pass that type around, so we can assume the ctor ran, but we aren't always assuming the deserialiser ran (such as what happens with tests)
@aaronpowell aaronpowell self-assigned this Jun 27, 2023
Base automatically changed from aaronpowell/new-config-engine to main June 29, 2023 23:11
@seantleonard seantleonard force-pushed the aaronpowell/new-config-engine-null-handling branch from cf26b97 to 4b97d51 Compare June 30, 2023 17:32
Copy link
Contributor

@Aniruddh25 Aniruddh25 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@seantleonard seantleonard left a comment

Choose a reason for hiding this comment

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

Two questions, but looks good!

@aaronpowell
Copy link
Contributor Author

This PR is reintroducing some stuff that it shouldn't, I'll have to check all the .NET changes again before it can be merged

It was removed in 11eb639 as it would cause the config to never get loaded (see original commit for details)
@aaronpowell aaronpowell enabled auto-merge (squash) July 4, 2023 05:47
@aaronpowell aaronpowell merged commit b636c96 into main Jul 6, 2023
@aaronpowell aaronpowell deleted the aaronpowell/new-config-engine-null-handling branch July 6, 2023 01:27
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