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

[GR-52611] Add the ability to add comments to JSON configuration file entries #8543

Closed
zakkak opened this issue Mar 8, 2024 · 8 comments
Closed

Comments

@zakkak
Copy link
Collaborator

zakkak commented Mar 8, 2024

Feature request

Similar to #7482, it would be nice to have the ability to include comments in all the JSON config files.

Is your feature request related to a problem? Please describe.

Right now there is no way to include comments justifying the choices made in configuration files.

Describe the solution you'd like.

Introduce a new key in the JSON schema to allow the addition of comments per entry.

Describe who do you think will benefit the most.

Everyone using the JSON configuration files.

Describe alternatives you've considered.

Dump a separate file with justifications.

Additional context.

I am planning to augment Quarkus to give the users more information about the reasons that lead to adding each entry in the json configuration files we generate (e.g. which dependency brings it in, which class hierarchy, etc.) and I think adding that info in the configuration file itself would be best.

Express whether you'd like to help contributing this feature

Yes.

@zakkak zakkak added the feature label Mar 8, 2024
@zakkak zakkak changed the title Add the ability to add comments to JSON configution file entries Add the ability to add comments to JSON configuration file entries Mar 8, 2024
@fernando-valdez fernando-valdez changed the title Add the ability to add comments to JSON configuration file entries [GR-52611] Add the ability to add comments to JSON configuration file entries Mar 9, 2024
@fernando-valdez
Copy link
Member

Created internal ticket: GR-52611

@fniephaus
Copy link
Member

When I read "comments in JSON", I thought you're asking that we extend our JSON infra to allow actual comments, which would probably be quite messy. Adding an optional reason property for every supported entry is probably a good idea. Do you think it should be reason or do we need reasons which accepts an array of strings?

@zakkak
Copy link
Collaborator Author

zakkak commented Mar 11, 2024

I believe reasons is more appropriate as there might be multiple reasons/comments to have a configuration present, e.g. more than one classes are reflectively accessing the class we are registering, etc.

@fniephaus
Copy link
Member

Agreed, and "reasons": ["only one reason"] is only two chars longer than "reason": "only one reason". 😉

@fniephaus
Copy link
Member

@zakkak I assume you are working on this? We just need to extend the schema files, right?

@zakkak
Copy link
Collaborator Author

zakkak commented Mar 14, 2024

@zakkak I assume you are working on this?

Not yet. Now that I have your ACK, I will probably get to it in late March or early May.

We just need to extend the schema files, right?

Yes, that's my understanding, plus some documentation probably.

@zakkak
Copy link
Collaborator Author

zakkak commented Jun 6, 2024

WIP PR #9048

@zakkak
Copy link
Collaborator Author

zakkak commented Jun 18, 2024

This feature was implemented in #9048.

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

Successfully merging a pull request may close this issue.

4 participants