[Maps] Onboarding maps-common & maps-search package
#23039
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Here is the API view created previously: https://apiview.dev/Assemblies/Review/737c63ff29ea4eb8ad335ddabb4c382f |
|
This pull request is protected by Check Enforcer. What is Check Enforcer?Check Enforcer helps ensure all pull requests are covered by at least one check-run (typically an Azure Pipeline). When all check-runs associated with this pull request pass then Check Enforcer itself will pass. Why am I getting this message?You are getting this message because Check Enforcer did not detect any check-runs being associated with this pull request within five minutes. This may indicate that your pull request is not covered by any pipelines and so Check Enforcer is correctly blocking the pull request being merged. What should I do now?If the check-enforcer check-run is not passing and all other check-runs associated with this PR are passing (excluding license-cla) then you could try telling Check Enforcer to evaluate your pull request again. You can do this by adding a comment to this pull request as follows: What if I am onboarding a new service?Often, new services do not have validation pipelines associated with them, in order to bootstrap pipelines for a new service, you can issue the following command as a pull request comment: |
dubiety
reviewed
Aug 31, 2022
andykao1213
force-pushed
the
andrewkao/maps-search
branch
from
eb39dbb to
6bbbbf5
Compare
|
API change check APIView has identified API level changes in this PR and created following API reviews. |
jeremymeng
approved these changes
Sep 23, 2022
There was a problem hiding this comment.
API surface looks good. @joheredi please take another look.
joheredi
reviewed
Sep 26, 2022
| BatchPollerProxy, | ||
| createAzureMapsKeyCredentialPolicy, | ||
| createMapsClientIdPolicy, | ||
| } from "../../maps-common/src"; |
There was a problem hiding this comment.
Shouldn't this be imported from @azure/maps-common?
There was a problem hiding this comment.
We recommend publishing @azure/maps-common. Using a local reference has brought us more problems than it is worth. If we publish the common package any major changes to it can be versioned which is a big advantage.
@timovv - fyi as he's familiar with the problems we've faced with sharing code this way.
There was a problem hiding this comment.
I see. I agree that the versioned package should bring benefits to us. Let me change this.
@timovv If you don't mind, could you share your experience of the problem you've encountered? 🤔
There was a problem hiding this comment.
@joheredi Now the @azure/maps-common has been changed to a published package, please take a look. Thank you.
There was a problem hiding this comment.
The main issue with a local reference is that making a breaking change to the local code means you must immediately change all the packages that depended on it. We had to do this with the Key Vault SDK a few months back, and it resulted in a giant PR which was very difficult to review. We ran into a couple of issues that we had to hotfix as a result of that which would have most likely been caught otherwise. Publishing means you can roll out those changes incrementally across your packages, which is a lot safer!
There was a problem hiding this comment.
@timovv Understood. Thanks for the detailed explanation 👍🏼
joheredi
reviewed
Sep 27, 2022
|
|
||
| // @public | ||
| export interface GeoJsonFeature extends GeoJsonObject { | ||
| // (undocumented) |
There was a problem hiding this comment.
make sure to document all these undocumented properties
There was a problem hiding this comment.
I've added the reference and summary from the GeoJSON spec to the related interface, and I believe the information should be enough. The rest of the undocumented properties is considered unnecessary from our side.
For the BatchPollerProxy, since it's for internal so I leave it undocumented. Please refer to the commit: fd7722f
joheredi
reviewed
Sep 27, 2022
| // (undocumented) | ||
| properties?: Record<string, any>; | ||
| // (undocumented) | ||
| type: "Feature"; |
There was a problem hiding this comment.
would it be possible to name these properties kind instead of type? It is more commonly used as a discriminator in our libraries
There was a problem hiding this comment.
This is the GeoJSON spec, so I think it would be better to keep the convention
joheredi
reviewed
Sep 27, 2022
| export class MapsSearchClient { | ||
| constructor(credential: AzureKeyCredential, options?: MapsSearchClientOptions); | ||
| constructor(credential: TokenCredential, mapsAccountClientId: string, options?: MapsSearchClientOptions); | ||
| beginFuzzySearchBatch(requests: FuzzySearchRequest[], options?: FuzzySearchBatchOptions): Promise<BatchPoller<BatchResult<SearchAddressResult>>>; |
There was a problem hiding this comment.
I wonder if we should call these queries instead of requests. @jeremymeng what do you think?
There was a problem hiding this comment.
That seems to be consistent with the non-batch functions
There was a problem hiding this comment.
But the requests is in the array-type of FuzzySearchRequest, which contains not only SearchQuery but also another property options. Would it be better to keep it requests to avoid confusion?
joheredi
reviewed
Sep 27, 2022
There was a problem hiding this comment.
Thanks for addressing the suggestions @andykao1213! I think this is looking pretty good, I'd like @deyaaeldeen to take a look a the LRO implementation as there is a new API that we may be able to leverage here. Other than that I left a few small comments but the API is looking really good.
deyaaeldeen
reviewed
Sep 28, 2022
| implements BatchPoller<TBatchResult> | ||
| { | ||
| constructor( | ||
| private internalPoller: PollerLike< |
There was a problem hiding this comment.
Does your LROs support cancellation?
There was a problem hiding this comment.
The internalPoller only accepts the generated poller (e.g. beginFuzzySearchBatch), which is a wrapper of LroEngine. But I see that the LroEngine.cancelOperation is deprecated. So should I remove this?
There was a problem hiding this comment.
@deyaaeldeen per the previous discussion, since map-search's lro doesn't support cancelation, I've hidden it in the commit 1de07d2.
I will open another PR before GA to replace the generated lro with createHttpPoller as you suggest.
- Remove the unnecessary rule setting of eslint - Update the legacy type path
andykao1213
changed the title
maps-search packagemaps-common & maps-search package
Remove this because maps-search's lro doesn't support cancelation. For future lro that support cancelation, we should use the `createHttpPoller` like the one in the [ai-language-text](https://github.com/Azure/azure-sdk-for-js/blob/cb198d21c4f377e9d2d57744523b8eb3f45a2fb6/sdk/cognitivelanguage/ai-language-text/src/textAnalysisClient.ts#L686)
joheredi
approved these changes
Sep 30, 2022
andykao1213
added a commit
that referenced
this pull request
Oct 26, 2022
* Update maps dependency in pnpm-lock.yaml * Fix sample metadata errors * Fix linting errors * Fix linting errors * Fix run time error * Update package.json in samples folder * Fix linting errors * Fix linting errors * Add changelog for maps-common * Update recording files for maps-search * Add test.yml for maps-search * Add tests.yml for render and route * Update tests.yml * Update test-resources.json for maps * Fix tests.yml format error * Update env vars and remove unused export var * Remove unused code * Fix lock file * Remove route & render * Replace `beginGet*Batch` with `resume*Batch` We provide a `beginGet*Batch` method to resume the previous request previously. This is not ideal because it need to get a `batchId` which is store in the internal state of the LRO. Once the LRO brings changes, our code will be broken. We can achieve the same thing by using serialized state. This change aims to leverage this approach and create convenience methods `resume*Batch` for users * Update recording * Make the endpoint of `MapsSearchClient` configurable * Change LatLon from an interface to a tuple Also remove the `toLatLon` function since creating a tuple is quite easy. * Make the props `coordinates` and `countryCodeFilter` optional Specifying `coordinates` and `countryCodeFilter` can narrow the results of the fuzzy search. But considring the case like searching "Australia", theese fields may have no use. We decided to make them optional so user can decides if they want to narrow down the results or not. * fixup! Remove route & render * Adjust CHANGELOGs of maps-search & common Following the keyvault-common, removing CHANGELOGs from maps-common since it's not published. * Update samples * Add batchRequest sample * Add a new sample searchAddressResult * Update search.ts and its examples * enhance code example in README.md * comment unused import * Update recording for maps-search * remove redundant recording files * Add maps-search * Rename LatLong to LatLon * Fix constructor Readme doc * Update readme to new signature * Add tests partially * Change LatLon/BoundingBox to interfaces * Reduce the number of constructor overloading * Rename SearchClient as MapsSearchClient * Add options mapper tests * Add result type mapper tests (in progress) * rename clientId to mapsAccountClientId * listPolygons -> getPolygons * getPointOfInterestCategoryTree -> getPointOfInterestCategories * Expose missing model * Add alternative input file for quick experiment * Regenerate code and make changes accordingly * Reduce number of overloads of fuzzy searches * Add geojson types * Adopting new geojson definitions * Change test specs to align with new method names * clean up models and tsconfig * Expose Known* models * Remove batch sync methods * Add public tests (non-batch) * move model * Add docstrings * Start polling in begin* method * Refactor BatchPoller for retrieving batchId * Finalize batch pollers * Readd batch methods * Update generated code * Finalize batch signature * Rebuild samples & clean up * Add AAD tests * Clean up Swagger readme * Fix spelling * Add initial files for @azure/maps-route module * maps-search: change comments and perform formatting * Methods options and testing skeleton for maps-route * maps-search: refactor options * Improve convenience layer for non-LRO methods * Change variable names * Complete batch methods * update maps-search Readme * Update maps-route Readme * Add generated samples for maps-route * Update samples for maps-search * Delete backup file * Add maps-common module * Update maps-route/maps-search to use maps-common * maps-render: first draft * Use LatLon/BoundingBox from maps-common * Complete maps-render with tests * Expose required types from maps-route * Expose required types from maps-search * Complete unit tests for maps-render * Update maps-render README * Update samples * Fix comment/strings * Fix comment/strings * Update Route comment/interface * Regenerate code * Update files * Define required fields and fix code * Remove non-poller batch methods * Fix playback test failure * Rename parts of model names * Rename remaining parts of models according to review feedback * Remove TODO comments * Update Reamdme transform to reflect latest swagger * Change key policy name * Simplify batch item error model * Add brower tests * Turn maps-common into internal module * Rearrange Readme * Rewrite Readme for common * Remove batch-sync methods * Route required fields: part 1 * Route required fields: part 2 * Add AAD back * Route model fixes #1 * Route model fixes #2 + geojson named tuples * Refine geojson types * Aliased a few method options * maps-render required fields part 1 * Do not validate budget parameters * Revert "Do not validate budget parameters" This reverts commit 4ec3f5f. * Rename MapTileset to TileJson * Render: model rename & type improvement * Explicitly transform image range * Fix route record tests * Update map account creation commands in Readme * Update Readme/docstrings/samples for Search/Route/Render * Remove rollup and use dev-tool instead * Remove some empty tests * Add AAD test case and make file name shorter * Update test cases * Update cspell rules * Update recording files * Simplify test cases * Sync cspell.json from upstream * Use test-recorder 2.0 and update unit test cases * Push wrong browsers recording files * Update settings to support recording * Fix dependency errors * Upgrade api-recorder and fix errors * Code refinement * Update maps search recording files * Update karma.conf.js format * Update recording files and settings * Fix broken links * Update maps dependency in pnpm-lock.yaml * Fix sample metadata errors * Fix linting errors * Fix linting errors * Fix run time error * Update package.json in samples folder * Fix linting errors * Fix linting errors * Add changelog for maps-common * Update recording files for maps-search * Add test.yml for maps-search * Add tests.yml for render and route * Update tests.yml * Update test-resources.json for maps * Fix tests.yml format error * Update env vars and remove unused export var * Remove unused code * Fix lock file * Remove maps-render * fixup! Change LatLon from an interface to a tuple * Comment the future doc link * Update doc & fix type issue * fix lint issues * Regenerate API View * Comment the whole [apiref] since the original syntax cannot pass the Analyze/Link verification * Fix issue related to the change of maps-common - Replace beginGet*Result with resume* methods - Turn LatLon to a tuple - Add mapper to transform the string to Date because resume* method will return a serialized Date * Update test recording * Remove maps-common from the Artifacts * remove unused dependencies * Add impression in README so we can track the page views * Remove `AZURE_LOG_LEVEL` in test.yml * Update samples - Move the option parameter to the right place - Add comments & "break" for handling the type of the search result to make it clear for the user * Patch {} to properties for GeoJsonPolygon * rename maxDetourTime to maxDetourTimeInSeconds * Update recordings * Remove any * Rename to detourTimeInSeconds to align the convention * Change the test.yml EnvVar See more explanation here: - #23039 (comment) - #23039 (comment) * Rename numResults to numberResults * Add document comment * Add endpoint options to MapsRouteClient * Remove unused dependencies in maps-route * fixup! Change the test.yml EnvVar * Update interfaces & documents * Update changelog * Merge getRouteDirection with the additional parameter one * Update samples * Make the `maps-common to a published package * Update the import of `maps-common` in `maps-search` * Fix CI Issue * Add documents for geojson * Update the samples & readme * Leverage maps-common package in mpas-route * chore update - Remove the unnecessary rule setting of eslint - Update the legacy type path * Remove unnecessary eslint config * WIP: Remove LRO generation * WIP: refactor mapsRouteClient * WIP: first generation * Format the generated code * Support AAD * Transform the model name * Add helper functions * Add a sample - route.ts * Add samples for lro * Add test * Generate samples * Add api view * Update the generation * WIP: ReadMe * Rename "computeBestOrder" to "computeBestWaypointOrder" * Refine README * Rename model * Add test createRouteDirectionsBatchRequest * Fix lint issue * Add missing metadata * Remove maps-route/ * Update CI issue * Fix merged issue * Update version rule for maps-common * Update CI name * Remove extra lint config * Use `require` in the README for consistency * Regenerate the samples * Remove redundant recording files * Remove unused parameters in test-resources.json * Refine readme snippet * Log something specific for the result * Update samples * Split the route.ts into 3 * Remove model rename * Use default export for RLC factory function Co-authored-by: Charlie Chen <[email protected]> Co-authored-by: Charlie Chen <[email protected]> Co-authored-by: --global <[email protected]>
mpodwysocki
pushed a commit
that referenced
this pull request
Nov 2, 2022
* Update maps dependency in pnpm-lock.yaml * Fix sample metadata errors * Fix linting errors * Fix linting errors * Fix run time error * Update package.json in samples folder * Fix linting errors * Fix linting errors * Add changelog for maps-common * Update recording files for maps-search * Add test.yml for maps-search * Add tests.yml for render and route * Update tests.yml * Update test-resources.json for maps * Fix tests.yml format error * Update env vars and remove unused export var * Remove unused code * Fix lock file * Remove route & render * Replace `beginGet*Batch` with `resume*Batch` We provide a `beginGet*Batch` method to resume the previous request previously. This is not ideal because it need to get a `batchId` which is store in the internal state of the LRO. Once the LRO brings changes, our code will be broken. We can achieve the same thing by using serialized state. This change aims to leverage this approach and create convenience methods `resume*Batch` for users * Update recording * Make the endpoint of `MapsSearchClient` configurable * Change LatLon from an interface to a tuple Also remove the `toLatLon` function since creating a tuple is quite easy. * Make the props `coordinates` and `countryCodeFilter` optional Specifying `coordinates` and `countryCodeFilter` can narrow the results of the fuzzy search. But considring the case like searching "Australia", theese fields may have no use. We decided to make them optional so user can decides if they want to narrow down the results or not. * fixup! Remove route & render * Adjust CHANGELOGs of maps-search & common Following the keyvault-common, removing CHANGELOGs from maps-common since it's not published. * Update samples * Add batchRequest sample * Add a new sample searchAddressResult * Update search.ts and its examples * enhance code example in README.md * comment unused import * Update recording for maps-search * remove redundant recording files * Add maps-search * Rename LatLong to LatLon * Fix constructor Readme doc * Update readme to new signature * Add tests partially * Change LatLon/BoundingBox to interfaces * Reduce the number of constructor overloading * Rename SearchClient as MapsSearchClient * Add options mapper tests * Add result type mapper tests (in progress) * rename clientId to mapsAccountClientId * listPolygons -> getPolygons * getPointOfInterestCategoryTree -> getPointOfInterestCategories * Expose missing model * Add alternative input file for quick experiment * Regenerate code and make changes accordingly * Reduce number of overloads of fuzzy searches * Add geojson types * Adopting new geojson definitions * Change test specs to align with new method names * clean up models and tsconfig * Expose Known* models * Remove batch sync methods * Add public tests (non-batch) * move model * Add docstrings * Start polling in begin* method * Refactor BatchPoller for retrieving batchId * Finalize batch pollers * Readd batch methods * Update generated code * Finalize batch signature * Rebuild samples & clean up * Add AAD tests * Clean up Swagger readme * Fix spelling * Add initial files for @azure/maps-route module * maps-search: change comments and perform formatting * Methods options and testing skeleton for maps-route * maps-search: refactor options * Improve convenience layer for non-LRO methods * Change variable names * Complete batch methods * update maps-search Readme * Update maps-route Readme * Add generated samples for maps-route * Update samples for maps-search * Delete backup file * Add maps-common module * Update maps-route/maps-search to use maps-common * maps-render: first draft * Use LatLon/BoundingBox from maps-common * Complete maps-render with tests * Expose required types from maps-route * Expose required types from maps-search * Complete unit tests for maps-render * Update maps-render README * Update samples * Fix comment/strings * Fix comment/strings * Update Route comment/interface * Regenerate code * Update files * Define required fields and fix code * Remove non-poller batch methods * Fix playback test failure * Rename parts of model names * Rename remaining parts of models according to review feedback * Remove TODO comments * Update Reamdme transform to reflect latest swagger * Change key policy name * Simplify batch item error model * Add brower tests * Turn maps-common into internal module * Rearrange Readme * Rewrite Readme for common * Remove batch-sync methods * Route required fields: part 1 * Route required fields: part 2 * Add AAD back * Route model fixes #1 * Route model fixes #2 + geojson named tuples * Refine geojson types * Aliased a few method options * maps-render required fields part 1 * Do not validate budget parameters * Revert "Do not validate budget parameters" This reverts commit 4ec3f5f. * Rename MapTileset to TileJson * Render: model rename & type improvement * Explicitly transform image range * Fix route record tests * Update map account creation commands in Readme * Update Readme/docstrings/samples for Search/Route/Render * Remove rollup and use dev-tool instead * Remove some empty tests * Add AAD test case and make file name shorter * Update test cases * Update cspell rules * Update recording files * Simplify test cases * Sync cspell.json from upstream * Use test-recorder 2.0 and update unit test cases * Push wrong browsers recording files * Update settings to support recording * Fix dependency errors * Upgrade api-recorder and fix errors * Code refinement * Update maps search recording files * Update karma.conf.js format * Update recording files and settings * Fix broken links * Update maps dependency in pnpm-lock.yaml * Fix sample metadata errors * Fix linting errors * Fix linting errors * Fix run time error * Update package.json in samples folder * Fix linting errors * Fix linting errors * Add changelog for maps-common * Update recording files for maps-search * Add test.yml for maps-search * Add tests.yml for render and route * Update tests.yml * Update test-resources.json for maps * Fix tests.yml format error * Update env vars and remove unused export var * Remove unused code * Fix lock file * Remove maps-render * fixup! Change LatLon from an interface to a tuple * Comment the future doc link * Update doc & fix type issue * fix lint issues * Regenerate API View * Comment the whole [apiref] since the original syntax cannot pass the Analyze/Link verification * Fix issue related to the change of maps-common - Replace beginGet*Result with resume* methods - Turn LatLon to a tuple - Add mapper to transform the string to Date because resume* method will return a serialized Date * Update test recording * Remove maps-common from the Artifacts * remove unused dependencies * Add impression in README so we can track the page views * Remove `AZURE_LOG_LEVEL` in test.yml * Update samples - Move the option parameter to the right place - Add comments & "break" for handling the type of the search result to make it clear for the user * Patch {} to properties for GeoJsonPolygon * rename maxDetourTime to maxDetourTimeInSeconds * Update recordings * Remove any * Rename to detourTimeInSeconds to align the convention * Change the test.yml EnvVar See more explanation here: - #23039 (comment) - #23039 (comment) * Rename numResults to numberResults * Add document comment * Add endpoint options to MapsRouteClient * Remove unused dependencies in maps-route * fixup! Change the test.yml EnvVar * Update interfaces & documents * Update changelog * Merge getRouteDirection with the additional parameter one * Update samples * Make the `maps-common to a published package * Update the import of `maps-common` in `maps-search` * Fix CI Issue * Add documents for geojson * Update the samples & readme * Leverage maps-common package in mpas-route * chore update - Remove the unnecessary rule setting of eslint - Update the legacy type path * Remove unnecessary eslint config * WIP: Remove LRO generation * WIP: refactor mapsRouteClient * WIP: first generation * Format the generated code * Support AAD * Transform the model name * Add helper functions * Add a sample - route.ts * Add samples for lro * Add test * Generate samples * Add api view * Update the generation * WIP: ReadMe * Rename "computeBestOrder" to "computeBestWaypointOrder" * Refine README * Rename model * Add test createRouteDirectionsBatchRequest * Fix lint issue * Add missing metadata * Remove maps-route/ * Update CI issue * Fix merged issue * Update version rule for maps-common * Update CI name * Remove extra lint config * Use `require` in the README for consistency * Regenerate the samples * Remove redundant recording files * Remove unused parameters in test-resources.json * Refine readme snippet * Log something specific for the result * Update samples * Split the route.ts into 3 * Remove model rename * Use default export for RLC factory function Co-authored-by: Charlie Chen <[email protected]> Co-authored-by: Charlie Chen <[email protected]> Co-authored-by: --global <[email protected]>
azure-sdk
pushed a commit
to azure-sdk/azure-sdk-for-js
that referenced
this pull request
Apr 18, 2023
Release machinelearningservices microsoft.machine learning services 2023 04 01 (Azure#23450) * Adds base for updating Microsoft.MachineLearningServices from version stable/2022-10-01 to version 2023-04-01 * Updates readme * Updates API version in new specs and examples * [Stable] Bring Back schedule shutdown MLC models (Azure#23039) * bring back schedule shutdown MLC models * add description --------- Co-authored-by: Naman Agarwal <[email protected]> * add minLength:1 prop to required strings (Azure#23109) * [Stable] Add IdleShutdown properties and api (Azure#23042) * [Stable] Add IdleShutdown properties and api * update version * remove api to update IdleShutdownSetting * Delete example --------- Co-authored-by: Naman Agarwal <[email protected]> * Pushing new features in stable swagger (Azure#23071) * Pushing new features in stable swagger * Added example file * Triggering retest * Fixed version issue * Triggering retest * Removing OS Patching * Changes as per feedback, and add os patching back * Fix formatting * Removed Custom Service Update API * Fixed Example Files * Added as per review comments * Added as per review comments * Retrigger test --------- Co-authored-by: Srivatsa Sinha <[email protected]> * Add Registries 2023-04-01 Swagger (Azure#23085) * add swagger + examples * fix readme * fix readme * switching to autogenerated registries swagger * fix spellcheck * update ga examples * previous API version for comparison = 2023-02-01-preview * new API version for comparison = 2023-04-01 * update examples * update swagger, examples to fix pr gates * fix lint diff errors * delete should have a 202 response, update createorupdate examples, fix removeRegions operation * removeRegions -> LRO POST operation * fix LRO options on removeRegions * add registryNameParamert * update swagger * update * PATCH property should have sku, identity properties * fix example --------- Co-authored-by: Komal Yadav <[email protected]> * MFE April 2023 staging (Azure#23164) * add latest mfe.json * updated examples * add wasbs to known words * swap pendingupload to proper naming * align RPs on Azure Machine Learning Services * rename pendingUpload to verb: startPendingUpload * remove trailing slash startpendingupload * improve descriptions * add pattern to all resource name types + examples * update original-uri -> azure-async-operation * remove wasbs and mark credential usage as secret * test is changing all to azure-async-operation causes breaking change * the code is implemented where the original-uri is where you will find the final object post-polling * update examples to remove wasbsUri * reference registries instead for RegistryName * remove wasbs * original-uri -> azure-async-operation * update examples to empty secrets * CreateOrUpdatePendingUpload -> CreateOrUpdateStartPendingUpload * add pattern for workspaceName * x-ms-secret on the SASUri * switch back to original-uri given update from auto-rest folks --------- Co-authored-by: Kayla Ames <[email protected]> * update compute examples to ReadOnly * v5->v3 * PartialManagedServiceIdentity -> RegistryPartialManagedServiceIdentity * update ID for delete/cancel job/schedule location header * PrivateEndpointConnection -> RegistryPrivateEndpointConnection * more renames for Registry * align mlc with arm * update registry examples * update registryPrivateLinkServiceConnectionState in examples * update description arm id for registryPrivateEndpointConnections * update to preview --------- Co-authored-by: Naman Agarwal <[email protected]> Co-authored-by: Naman Agarwal <[email protected]> Co-authored-by: Teddy Todorov <[email protected]> Co-authored-by: srivatsasinha <[email protected]> Co-authored-by: Srivatsa Sinha <[email protected]> Co-authored-by: Komal Yadav <[email protected]> Co-authored-by: Komal Yadav <[email protected]> Co-authored-by: Kayla Ames <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Packages impacted by this PR
Issues associated with this PR
Describe the problem that is addressed by this PR
What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen?
Are there test cases added in this PR? (If not, why?)
Provide a list of related PRs (if any)
Command used to generate this PR:**(Applicable only to SDK release request PRs)
Checklists