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

[FEATURE REQ] Remove unnecessary package references for net 6+ TFM #41628

Open
thompson-tomo opened this issue Jan 27, 2024 · 4 comments
Open
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. EngSys This issue is impacting the engineering system. needs-team-attention This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that

Comments

@thompson-tomo
Copy link

Library name

Azure.Core

Please describe the feature.

I want package references to be framework specific so that i am not adding in un-necessary packages when functionality can be and should be provided by the framework reference. An example of such is System.Text.Json which is included in net 6

@github-actions github-actions bot added customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-triage This is a new issue that needs to be triaged to the appropriate team. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Jan 27, 2024
thompson-tomo added a commit to thompson-tomo/azure-sdk-for-net that referenced this issue Jan 27, 2024
@thompson-tomo thompson-tomo changed the title [FEATURE REQ] Reduce Packages references for net 6+ TFM [FEATURE REQ] Reduce Azure.COre Packages references for net 6+ TFM Jan 27, 2024
@thompson-tomo thompson-tomo changed the title [FEATURE REQ] Reduce Azure.COre Packages references for net 6+ TFM [FEATURE REQ] Reduce Azure.Core Packages references for net 6+ TFM Jan 27, 2024
@jsquire jsquire added Client This issue points to a problem in the data-plane of the library. EngSys This issue is impacting the engineering system. needs-team-attention This issue needs attention from Azure service team or SDK team and removed needs-triage This is a new issue that needs to be triaged to the appropriate team. labels Jan 27, 2024
@jsquire jsquire changed the title [FEATURE REQ] Reduce Azure.Core Packages references for net 6+ TFM [FEATURE REQ] Remove unnecessary package references for net 6+ TFM Jan 27, 2024
@jsquire
Copy link
Member

jsquire commented Jan 27, 2024

@jsquire
Copy link
Member

jsquire commented Jan 27, 2024

Hi @thompson-tomo. Thank you for opening this issue. We've been having a variation of this discussion for quite some time, but with a focus on adding additional TFMs to all packages. Thus far, we've not been able to reach consensus on that - but as this request is more narrowly scoped, I believe it's a good point for additional consideration.

Proposal

This is something that I believe would be beneficial to all Azure SDK packages, and something that we could address by updating the common build targets to remove references for packages known to be unnecessary at build time for those platforms. This would allow us to avoid the burden of downloading unnecessary polyfill packages for the majority of supported platforms.

I'd appreciate thoughts from the folks mentioned above as well as the architects. As this is a recurring request and has been in discussion for a very long time, it is something that we need to have a final decision on.

Questions and challenges

  • Should we apply this to just the packages which already have explicit net6.0 targets, such as Core and Identity, to avoid having a global net6.0 target?

  • Is it reasonable to detect when we're building for a known-safe platform in the common build targets so that we can tune references globally and automatically?

  • Should we add the net6.0 and net461 targets to the central EngSys so that we can apply this to all packages? (this has been under discussion for quite a while without a conclusion.
    Now that we have an official TFM lifecycle, this really needs a decision.)
    .

  • Would our existing test matrix be enough to prove that any implementation is safe, or would there need to be additional work done?

References and related

@thompson-tomo
Copy link
Author

thompson-tomo commented Jan 27, 2024

Hi @jsquire

Thank you for your response. I agree that this is something which would benefit the entire sdk but was being mindful of the amount of work it would take. Also I hadn't seen the PR (#28911) which you have referenced.

My thoughts on this topic would be to review the PR as it is with the goal of accepting it based on it providing an improvement/enhancement to the current situation. A seperate issue can be created to move to centrally managing packages but low priority based on benefit gained at this point in time.

Regarding adding additional TFM, I am of the belief the main reason to add a TFM is that you need to do something conditionally based on the TFM ie add/remove packages or framework specific code.

@heaths
Copy link
Member

heaths commented Jan 29, 2024

  • Should we add the net6.0 and net461 targets to the central EngSys so that we can apply this to all packages? (this has been under discussion for quite a while without a conclusion.
    Now that we have an official TFM lifecycle, this really needs a decision.)
    .

Where it gets interesting is: should we still retain netstandard2.0? I think we have to. While net461 will cover everything up to net6.0 - including .NET "Core", IIRC (we tested this a few months ago in the ongoing discussions) - I don't know if that applies to every target platform e.g., Mono, though. Also, there are a few "extra" APIs declared in netstandard2.0 that aren't actually in net461, though most of those are in System.Security.Cryptography which primarily impact the Key Vault SDKs, which I've already handled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. EngSys This issue is impacting the engineering system. needs-team-attention This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that
Projects
None yet
Development

No branches or pull requests

5 participants