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.

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

Core deferred binding feature #1676

Merged
merged 7 commits into from Jul 11, 2023
Merged

Conversation

liliankasem
Copy link
Member

@liliankasem liliankasem commented Jun 23, 2023

Cherry picking the core pieces of the deferred binding feature, leaving out all extension changes.

  • Introduce ModelBindingData models
  • Introduce the deferred binding attributes
  • Sdk generator changes to implement supportsDeferredBinding metadata property
  • Creation of new unit test project for worker extensions

resolves #1702

@surgupta-msft
Copy link
Contributor

Did first pass on the PR to check if all the needed files are included here -- looks good. Will do another pass for reviewing the PR.

Question - what is the plan for bringing in sdk.analyzers and other tests - specially GrpcFunctionDefinitionTests and Metadata generator tests as all the functionality is here but we can't add their tests at this point?

@liliankasem
Copy link
Member Author

Did first pass on the PR to check if all the needed files are included here -- looks good. Will do another pass for reviewing the PR.

Question - what is the plan for bringing in sdk.analyzers and other tests - specially GrpcFunctionDefinitionTests and Metadata generator tests as all the functionality is here but we can't add their tests at this point?

Thanks for confirming Surbhi, I mostly want to make sure everything we need is coming through, but doesn't hurt to do another PR pass just in case and we can address any additional feedback.

For analyzers, I'd like to bring those in with their own PR.
For those tests, a lot of them are tied to an extension so I would like to bring those in as we bring in each extension

@liliankasem liliankasem force-pushed the liliankasem/deferred-binding branch from 819d987 to 1ba086a Compare July 5, 2023 17:29
Copy link
Member

@brettsam brettsam left a comment

Choose a reason for hiding this comment

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

Mostly just a quick question about whether we need a public surface API review on this...

src/DotNetWorker.Core/CollectionModelBindingData.cs Outdated Show resolved Hide resolved
@liliankasem liliankasem force-pushed the liliankasem/deferred-binding branch 9 times, most recently from 29ffaee to 19531cc Compare July 11, 2023 18:11
@liliankasem liliankasem merged commit e453de8 into main Jul 11, 2023
25 checks passed
@liliankasem liliankasem deleted the liliankasem/deferred-binding branch July 11, 2023 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Release deferred binding feature
4 participants