Move Java runtimes into a separate repository

jianghaolu commented 8 years ago

Background

As Java SDK gets more customers looking at our AutoRest generated library, the ability to easily make changes to runtime in the SDK repo has become a blocker for quickly iterating through the development process. The demand to easily see how the runtime looks like from the SDK repo has also been voiced.

Problem

We have a runtime that lives in AutoRest today - propagating it to the SDK requires publishing the runtime libraries either locally or externally. This slows down the development and requires a bit of setup.

Goal

Making changes to runtime in both AutoRest and SDK for Java should be trivial and the effects of the change should be applied with minimal effort.

Solution

Having a separate repository for runtime and reference it in both AutoRest and SDK as submodules. We've discussed this a bit back in time but I would like to try it out experimentally on Java.

Update

Instead of using a submodule, a subtree will better fit the scenario, for the following reasons: 1) Non-Java developers or external developers will see the folder exactly the same as before. When a pull request is sent, they don't need to know this is a subtree. Potentially, the sub-tree structure only needs one person on the team to know about. 2) Subtrees are generally easier to mess with. And supports lower version of Git. 3) Many people don't like submodules.

jianghaolu commented 8 years ago

To fetch the submodules:

git submodule init
git submodule update --remote

After making changes to the files in submodule, to submit them,

# First time you need to setup remote 
cd ./ClientRuntimes/Java
git remote rename origin upstream
git remote add origin git@github.com:{fork}/azure-clientruntime-for-java.git
# Normal commit and push
git commit && git push

devigned commented 8 years ago

Let's this be a warning: https://codingkilledthecat.wordpress.com/2012/04/28/why-your-company-shouldnt-use-git-submodules/. Thar be dragons down this path.

Perhaps, repo would be better.

devigned commented 8 years ago

Or maybe git subtree

jianghaolu commented 8 years ago

Repo would break the simple "clone and build" scenario. Subtree looks good.

brodyberg commented 8 years ago

How has the current repo design slowed development? How much have things been slowed? Compared to what?
How is the current publish step slowing things down? Do we have an issue to track that? What steps are we taking to mitigate this?
What is the plan to guarantee the same level of quality, unit-testing and code coverage in this new repo?
What is the plan to handle the additional legal and continuous-integration requirements of the new repo?
How does this proposal take into consideration the value of having all runtimes in the same place for unified review of features/issues?
The proposal calls for two repos - where do issues go? Here or there ... or?
I brought this design up to @stankovski when I proposed a layout for my work and he expressly encouraged this unified repo as it currently stands - has guidance changed?
The proposal is to use submodules so as to not break the clone and build but what about packages?
Sharing source is mentioned but Java is a compiled language - a dynamic language seems like a better candidate for the shared source requirement surely.
This issue is labeled as an enhancement, how is that the case?

jianghaolu commented 8 years ago

Thanks for your concern @brodyberg. 1) When there is a need to change the runtime, we need to publish it to a local binary repo and add this repo in the SDK to test it. For example, in order to add a functionality in Java's client-runtime, you would need to publish this to an internal maven repository (assume it's already set up) and go to SDK repo, add this repository's url to pom.xml. Then you can test it out. Whoops, forgot to implement one method. Go back. Publish. Test. Whoops, forgot to check null. Go back. Publish. Test. Whoops, maven server down. 2) As stated in 1). This is not those issues you track. This is one of those issues you bear with it everyday until one day you can't stand it anymore and you say okay okay let's fix it. And this is what I'm doing. 3) It's just the old runtime code copied over. If the old runtime coverage was top notch, it still is; if it was barely tested, it's still barely tested. 4) As you may already noticed, Travis-CI is already set up. All the code in this Java repo, in particular, has gone through JSR review and approved by Microsoft legal. 5) The truth is, Java developers couldn't care less about what other runtimes are doing. I imagine the same thing for other languages. The pain is more about developers not knowing where the runtime is when they look at the SDK repo. They come into Azure SDK for Java, they look at the code, cool, on paging operations they are returning a PagedList. Okay, what's the definition of PagedList though? For now we are expecting developers to say, aha, they must be a repo called "AutoRest", right? Of course. 6) Per David's suggestion, we will have a version of runtime code created as sub-trees in both AutoRest and Azure-sdk-for-Java. Personally I would allow them to create issues wherever they want - we can handle the re-routing. The more interesting question is how do we handle pull requests towards these 2 repos regarding fixes to the runtime. I'm still not fully convinced, but I'm leaning towards hiding this sub-tree structure from external contributors - i.e. they get their PR merged, happy, we do the dirty work of syncing it with the actual repo that runtime lives in. 7) I'm sorry that I have no context about your discussions. As a matter of a fact, when AutoRest was under rapid development, I was also all-in for this structure, because having the runtimes living in AutoRest repo simplifies the process of developing for a code generator. But once the code generator solidifies, more development work happens in the SDK and this problem surfaces up. No guidance is the guidance forever. 8) I don't understand the question. Particularly in Java, "build" builds out packages. I don't see changes in release activity either since code is really not modified. 9) Not only. SDK for Node used to keep a duplicate copy of the runtime in their node_modules. (I'm pretty sure they are not doing it anymore. @amarzavery ) This is basically a manual sub-tree. 10) AutoRest is for developers. I think Java developers' life is greatly enhanced starting from the moment they land on https://github.com/Azure/azure-sdk-for-java. They see the code, can contribute easily, I call this enhancement.

jianghaolu commented 8 years ago

I've updated the PR to use subtree instead of submodule.

To possibly address your concern, you can take a look at the repo it will potentially look like: https://github.com/jianghaolu/AutoRest/tree/javartremoval/ClientRuntimes. The Java folder is still there, in fact, for external developers, they don't even need to know this is a subtree folder.

jianghaolu commented 8 years ago

@brodyberg Could you take a look at https://github.com/jianghaolu/AutoRest/tree/javartremoval/ClientRuntimes and see if it addresses any of your concerns? Thanks.

brodyberg commented 8 years ago

@jianghaolu https://github.com/jianghaolu/AutoRest/tree/javartremoval/ClientRuntimes I will, thanks

On Monday, April 25, 2016, Jianghao Lu notifications@github.com wrote:

@brodyberg https://github.com/brodyberg Could you take a look at https://github.com/jianghaolu/AutoRest/tree/javartremoval/ClientRuntimes and see if it addresses any of your concerns? Thanks.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/Azure/autorest/issues/957#issuecomment-214435900

brodyberg commented 8 years ago

@jianghaolu It's great that you moved to subtree over submodule. My concern was that the overall idea behind this change may create the impression (if not the fact) of multiple sources of truth, which could be confusing. This is why I asked about bugs, etc. You seem to be offering a solution to a general problem which could also be a positive. I say go ahead and we can at least learn from your experience.

jianghaolu commented 8 years ago

The repo is in https://github.com/azure/autorest-clientruntime-for-java. For the limited period of 3 weeks it's been working fine. Closing.

Azure / autorest