Revise the repo structure to group packages under related service #186
Conversation
| sdk\keyvault\azure-keyvault\README.md | ||
| sdk\keyvault\azure-keyvault\src\main | ||
| sdk\keyvault\azure-keyvault\src\test | ||
| sdk\keyvault\azure-keyvault\src\samples |
There was a problem hiding this comment.
This feels redundant and less clear to me. I personally prefer having management and client subdirectories.
There was a problem hiding this comment.
Would you have the client and management subfolders and also the whole package name that contains mgmt for the management libraries?
There was a problem hiding this comment.
I wouldn't have that. It isn't required for Java - we don't really care about directory naming under we get way down into the src\main\java directory. Anything above that we can do what we want.
If I was being really radical, I would also argue that things would be simpler if we had sdk\client\... and sdk\management\.... You wouldn't believe how many times I've tried to run a maven command in the root of the java repo to have it fail, only to realise some minutes later that I forgot to specify -f pom.client.xml because maven assumes it should run by default on pom.xml, which belongs to the management library.
There was a problem hiding this comment.
Yes I agree there is some redundancy but I don't think adding a client/management/data-plane sub- directory helps at all. In the end for all the keyvault libraries you will end up with something that looks like:
sdk\keyvault\azure-mgmt-keyvault-v2015_06_01
sdk\keyvault\azure-mgmt-keyvault-v2016_10_01
sdk\keyvault\azure-keyvault-complete
sdk\keyvault\azure-keyvault-core
sdk\keyvault\azure-keyvault-cryptography
sdk\keyvault\azure-keyvault-extensions
sdk\keyvault\azure-keyvault-test
sdk\keyvault\azure-keyvault-webkey
sdk\keyvault\azure-keyvault
As for building only mgmt vs data-plane there will always have to be a choice of which POM file to build if you don't want to build all.
There was a problem hiding this comment.
My suggestion would be:
sdk\keyvault\management
sdk\keyvault\client
sdk\keyvault\data-plane
There was a problem hiding this comment.
So if I'm following the conversation correctly, the distinction between mgmt, data-plane (track 1) and client (track 2) will be reflected in the package names rather than separate folders in the current proposal. Correct?
There was a problem hiding this comment.
That is my understanding. This trips me up as often in the Java repo we tend to go one layer deep in some cases, e.g. keyvault/data-plane is the root of the project, and within this is half a dozen child modules (one of which is azure-keyvault...which despite the name is not the primary module). The current restructuring proposal doesn't currently support this extra level and I'm not clear how this will be articulated in this flatter structure.
There was a problem hiding this comment.
@JonathanGiles correct me if I'm wrong but there appears to be 2 different purposes for the parent POM files in the directories like https://github.com/Azure/azure-sdk-for-java/tree/master/keyvault/data-plane, one is to share configuration and the other is to build all modules as a group. For the shared configuration part do you feel that there will be any additional shared configuration for a service grouping beyond just configuration in the repo wide parent POM? Similarly for building the group of service related modules is it interesting to build only the mgmt vs track1 vs track2 module?
As for the building of the different POM files would it make since to always have a pom.xml file that always builds everything under a given directory so by default everything will get built but if we feel we want to scope the build to a subset like client, then we also create a pom.client.xml and have that only build the client stuff, we could consume that and any other scoping pom files in pom.xml which is the default pom the tools?
The worry I have is by placing these directories adjacent to each other, visitors to the repo will have no way of discerning the different between
microsoft-azure-keyvaultandazure-keyvault.
I have the same exact worry for our customers trying to consume these packages/jar files. We have to come up with good naming that makes sense across both our consuming customers context as well as our repo context. I don't think having a folder named "data-plane" and "client" make this any clearer for our visitors.
There was a problem hiding this comment.
Personally I would not take this approach (flattening the hierarchy to put all modules into a single directory per service, and introducing more non-standard pom files to reintroduce the structure that was removed).
There is definitely configuration that will happen at the parent level for some services, so we will need to continue to enable that.
Having a pom.xml that builds everything (mgmt, track 1 and 2) will be difficult - they will derive from separate repo root poms where dependencies, configuration, build tools, etc are vastly different. I believe a service-level pom will be largely pointless, so we will end up with pom.mgmt.xml, pom.data-plane.xml, and pom.client.xml, and we will be introducing the concept of different SDKs to users through requiring them to always specify a certain file to maven, rather than requiring them to navigate one level deeper (where they will find only the directories related to their area of interest).
However, I appreciate that there are differing perspectives here, and I don't seem to have the right words to convey that it feels to me like we're moving further away from the right solution :-) If this is the path you decide to take, I'll try my best to make things work - but it is probably a useful exercise to do a trial refactoring of what exists today in the repo, to see how it feels on the other side (and before track two arrives).
There was a problem hiding this comment.
There are 3 - management, client and runtime libraries
|
@lmazuel could you please take a look at this revised proposal to see if you feel like there would be any issues in the python repo? |
weshaggard
force-pushed
the
AddRepoDoc
branch
from
15ab609 to
0c87efa
Compare
|
Ok I chatted with @lmazuel and he is ok with the proposal which means I got sign-off from representatives from each language so I consider the proposal approved now, so merging this PR. We will use all the issues filed in each repo to track updating them to match the convention. |
After receiving more feedback I think it is better to group the packages under which service they apply to. This allows for each language to have a consist
sdk\<service name>set of directories and it keeps all packages related to a particular service grouped together. I'd like to hear any further feedback from the various language teams before we update the associated issues and make this change.https://github.com/azure/azure-sdk-for-net @shahabhijeet @AlexGhiondea -> Azure/azure-sdk-for-net#5213
https://github.com/azure/azure-sdk-for-java @jianghaolu @mayurid -> Azure/azure-sdk-for-java#2847
https://github.com/azure/azure-sdk-for-js @daschult @AlexGhiondea -> Azure/azure-sdk-for-js#1138
https://github.com/azure/azure-sdk-for-python @lmazuel @mayurid -> Azure/azure-sdk-for-python#4319
cc @Azure/azure-sdk-team