Skip to content

.Net: Obsolete SKContext[...] indexer#2188

Merged
shawncal merged 4 commits intomicrosoft:mainfrom
shawncal:skcontext-indexer-removal
Jul 26, 2023
Merged

.Net: Obsolete SKContext[...] indexer#2188
shawncal merged 4 commits intomicrosoft:mainfrom
shawncal:skcontext-indexer-removal

Conversation

@shawncal
Copy link
Contributor

Motivation and Context

The SKContext[...] indexer is simply shorthand for SKContext.Variables[...]. With additional routes to doing the same thing, there's more confusion about which to use when. In this case, there's a great deal of confusion about the mutability of these variables and whether they can/should be changed within functions or between functions.

Description

This PR starts to address the above issues by simplifying the SDK surface. The first step is to remove the "extra" route to accessing the same data, the indexer on SKContext.

This is a breaking change. The mitigation is simply to update all uses of context["MY_VALUE"] to context.Variables["MY_VALUE"]

In the PRs that follow, the mutability issue will be addressed by making it clear which variables are input, how and where to set output, and which can/cannot be transformed by an SKFunction.

Contribution Checklist

@shawncal shawncal added .NET Issue or Pull requests regarding .NET code kernel.core PR: breaking change Pull requests that introduce breaking changes labels Jul 26, 2023
@shawncal shawncal requested a review from a team as a code owner July 26, 2023 00:23
@shawncal shawncal added the kernel Issues or pull requests impacting the core kernel label Jul 26, 2023
stephentoub
stephentoub previously approved these changes Jul 26, 2023
rogerbarreto
rogerbarreto previously approved these changes Jul 26, 2023
@shawncal shawncal dismissed stale reviews from rogerbarreto and stephentoub via 0f09cbb July 26, 2023 16:23
@shawncal shawncal requested a review from rogerbarreto July 26, 2023 16:24
rogerbarreto
rogerbarreto previously approved these changes Jul 26, 2023
@shawncal shawncal added this pull request to the merge queue Jul 26, 2023
Merged via the queue into microsoft:main with commit 32447c0 Jul 26, 2023
@shawncal shawncal deleted the skcontext-indexer-removal branch July 26, 2023 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kernel Issues or pull requests impacting the core kernel .NET Issue or Pull requests regarding .NET code PR: breaking change Pull requests that introduce breaking changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants