search

Azure / durabletask

Durable Task Framework allows users to write long running persistent workflows in C# using the async/await capabilities.
Apache License 2.0
1.51k stars 290 forks source link

Merge core entities to Azure Storage Track2 #1012

Closed nytian closed 4 months ago

nytian commented 10 months ago

As titled. This PR is to merge feature core entities #960 to branch azure-storage-v12 for df v3 support.

nytian commented 9 months ago

Please Look at this commits for the changes: integrate with track2 https://github.com/Azure/durabletask/pull/1012/commits/404c8a7d3b4bb93b824deb3df5f5f7e4e55138eb

sebastianburckhardt commented 9 months ago

This is a bit difficult to review since it is based on feature/core-entities, which is a stale and very large branch. feature/core-entities has already been merged into main (and entities have received further updates since then).

What is the reason for merging the feature branch for core-entities, as opposed to just merging main? I am assuming you want to eventually merge main. I can see that main was merged into azure-storage-v12 last on September 9.

nytian commented 9 months ago

@sebastianburckhardt Thanks for the review. Sorry for the late reply and confuse. Actually, this PR merge with main branch, since it includes all the changes from main branch not just core entities. I will update the name since it looks confusing.

And only core entities need to be changed since it involves Azure Storage. So, the commit I asked for review is where I did the change for core entities to be merged to AS track2.

sebastianburckhardt commented 9 months ago

I am still a bit confused. I am happy to review your changes relative to main, but what I am seeing here are changes of main relative to your feature branch. This is a huge diff, most of which is not going to be relevant to your feature branch. But since I am not familiar with what is in your feature branch, I cannot quite tell what I should be looking at. Most of it just looks like changes we already reviewed and approved before merging the entities branch into main.

davidmrdavid commented 9 months ago

Maybe the solution here is to split this PR into two branches: (1) branch1- A branch that does a naive git pull of the changes in "main" as-is (2) branch2 -A fork of branch1 that does the "track2 AzStorage SDK" changes.

Then we can create a PR trying to merge branch2 into branch1 (this is the new code). This is the PR we'll review.

And from there we can open a routine PR trying to merge branch1 into the azure-strage-v12 branch. That will contain a lot of commits, but it'll be all already approved code so it should not require a thorough review, just a skim.

nytian commented 4 months ago

Close this as changes made in another PR #1081 and #1077