-
Notifications
You must be signed in to change notification settings - Fork 246
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
Standardize version property names #2460
Conversation
I feel like moving the properties this direction is a mistake. It makes us lose the implied grouping of the microservices. Id be fine with changing it to version.somethingBesidesMicroservervices.* if we don't like using that name across all the repositories, but I don't like losing the information that this was part of one of our services. How does a new developer differentiate one of our microservices from something we don't build? We already get confusion between version.accumulo and version.microservices.accumulo-utils, removing microservices will only make this worse. |
Luke and I talked about this before he started working on it. At the time, I had bought into the idea of removing the 'microservice' part of the property with the idea being that the property names were probably unique enough without that part, and that keeping things short would be nice. I can see the benefit of keeping a common prefix in the property name - having the properties grouped together is certainly nice. Maybe we should consider something other than 'microservice'. What about 'version.datawave.____' instead? Keep in mind that not all of the properties prefixed with 'microservice' were actually microservices (e.g. type-utils, metadata-utils, accumulo-utils, etc.). You could argue that the various api jars are not really 'microservice' jars either as they're depended on by both the microservices and datawave. The overall goal of this effort was to standardize the names across all projects so that we could update the version of a thing all at once. This will facilitate being able to run a full snapshot build of all of the services in a github action, which i think would be good to do. What are both of your thoughts on removing 'microservice' and instead using 'datawave' since these are all datawave artifacts? I think that would give everyone what they want. |
I think going with 'version.datawave.____' makes sense to me. |
I agree that version.datawave._____ works. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it a lot. Can you link the related PRs for our other repos so we can merge all of these at once? (assuming you've finished those already)
I linked all of the related PRs to the initial issue, there should be a list there (#2436) |
4dad491
Hey! let's get this wrapped up if we can. I checked out your stuff and ran the following command, and got this output.
If you look through that, I want you to ignore the different version numbers for the same property. I'm not really concerned about that. What I want you to focus on are the property name differences (e.g. version.datawave.authorization-api vs version.datawave.authorization.api) Here are the ones I noticed: authorization-api
base-rest-responses
hazelcast-common
mapreduce-query-core-job
metrics-reporter
|
Thanks for finding these. I made the corrections for the inconsistent property names mentioned above. Should all be good now. |
This PR is part of the effort to standardize version property names across all repos to make it easier to update the version of an artifact across the board.
Also updates documentation in the BUILDME regarding property names.
Related to: DW Issue 2436