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.

Sign up for GitHub

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

Jump to bottom

Core system properties cleanup #5056

Merged
merged 7 commits into from May 17, 2024
Merged

Core system properties cleanup #5056

merged 7 commits into from May 17, 2024

Conversation

vietj
Copy link
Member

@vietj vietj commented Jan 9, 2024 •
edited

  • introduce an enum representing core system properties that can be accounted for
  • vertx.json.base64 removal
  • remove cluster manager system property
  • remove properties having a corresponding option which are not documented
    • vertx.javaCompilerOptions
    • vertx.disableTCCL
  • deprecate system properties having a corresponding option which are documented
    • discuss with @tsegismont the handling of file system properties as explained in the documentation
  • consider reifying system properties as env variable like vertxweb.environment
  • remaining properties analysis

@vietj vietj added this to the 5.0.0 milestone Jan 9, 2024
@vietj vietj force-pushed the sysprops-cleanup branch 2 times, most recently from 785a0de to 80136e4 Compare January 9, 2024 13:34
@vietj vietj linked an issue Jan 9, 2024 that may be closed by this pull request
Undocumented system properties analysis and cleanup #4963
Closed
@vietj
Copy link
Member Author

vietj commented Jan 25, 2024

@tsegismont do you mind having a look at the system properties we do have ? I'm thinking at the cluster manager declaration at least that could go away in 5.0

tsegismont
tsegismont approved these changes Jan 29, 2024
View reviewed changes
Copy link
Contributor

@tsegismont tsegismont left a comment

Choose a reason for hiding this comment

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

It's fine to remove the cluster manager related sysprops, I think they only made sense when several cluster manager implementations where available on the CP (e.g. CLI with full profile, but the CLI is going away)

vietj added 5 commits May 17, 2024 13:49
@vietj
Gathers all system properties in a single place with a summary of the…
65823bc 
… scope of each
@vietj
Vert.x 3.x Json supports RFC-7493, however the JSON encoder/decoder f…
c0cf29b 
…ormat was incorrect. Users who needed to interop with Vert.x 3.x applications should have set the system property vertx.json.base64 to legacy.

This system property and mode is removed in Vert.x 5
@vietj
Remove vertx.cluster.managerClass system property
cc18145
@vietj
Remove vertx.javaCompilerOptions system property that is not document…
a03289a 
…ed nor tested
@vietj
Remove vertx.disableTCCL property
b589339
@vietj vietj marked this pull request as ready for review May 17, 2024 12:25
@vietj vietj merged commit ba5acd2 into master May 17, 2024
7 checks passed
@vietj vietj deleted the sysprops-cleanup branch May 17, 2024 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tsegismont tsegismont tsegismont approved these changes

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
5.0.0
Development

Successfully merging this pull request may close these issues.

Undocumented system properties analysis and cleanup
2 participants
@vietj @tsegismont