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

Adding global usings #2934

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Conversation

mattchenderson
Copy link
Contributor

@mattchenderson mattchenderson commented Jan 25, 2025

Adding support for global usings as part of the Functions SDK, based on what we see in the .NET SDK. This would allow users to remove usings from their projects, and we can update the templates to remove them as well.

Creating as a draft, as there are a few questions to resolve, and the PR checklist itself can't be fully completed yet.

Discussion of PR checklist topics

I think documentation changes would be needed here to at least cover the version requirement and generally set context The change would allow us to trim some usings from code snippets, but I am not sure if we should do that. Due to other versions still being extant, I think it may be appropriate to keep those, at least for some time. Leaving them will create some copy/paste of unnecessary lines, which isn't too big a concern. However, I'm open to making some targeted adjustments where we see fit.

To that end, if we take this forward, we'll want to plan on templates changes which correspond to it. The trick to those is that the item templates can be slightly divorced from the Functions worker SDK reference. That should be manageable (maybe through post-actions), but we would need to evaluate the scenarios there to ensure we don't drop usings in situations where these changes aren't present.

I will update the release notes as part of an update of the PR before it is ready for review.

Pull request checklist

  • My changes do not require documentation changes
  • My changes should not be added to the release notes for the next release
    • Otherwise: I've added my notes to release_notes.md
  • My changes do not need to be backported to a previous version
    • Otherwise: Backport tracked by issue/PR #issue_or_pr
  • I have added all required tests (Unit tests, E2E tests)

@mattchenderson
Copy link
Contributor Author

The test failure does not repro locally in test explorer, and I'm not sure why yet. But it highlights an interesting dependency challenge: the SDK itself isn't what defines Microsoft.Azure.Functions.Worker.Builder. That actually comes from the worker package. So, when that package is behind, this creates a failure. I believe that is what's happening in the test failure, though again, the fact that I don't see that locally might call that theory into question.

Regardless, there is an opportunity to update the test project, so I'm going to try that out.

@mattchenderson
Copy link
Contributor Author

With that test now passing (and no others seemingly impacted by the change), we are left with a question: Should <Using Include="Microsoft.Azure.Functions.Worker.Builder" /> actually be moved over to the Worker project? I'm thinking yes, so we avoid the build error that results from not having a 2.x version of the worker package. In fact, we probably should move both packages there. The SDK will just reference the Microsoft.Extesnions.* packages.

@mattchenderson mattchenderson marked this pull request as ready for review January 27, 2025 23:48
@jviau
Copy link
Contributor

jviau commented Jan 30, 2025

@mattchenderson I am not sure I agree with including all these usings. It will pollute IntelliSense and reduce customer's ability to finely control their usings. It also may lead to increased ambiguous resolutions if the same type name is found between these namespaces and other ones the customer may already be bringing in (which might be a compilation breaking change actually).

I wonder if customers can already get this behavior by changing their SDK?

What happens if you create a function app with <Project Sdk="Microsoft.NET.Sdk.Worker" />? or <Project Sdk="Microsoft.NET.Sdk.Web" />?

@mattchenderson
Copy link
Contributor Author

Based on offline discussion with Jacob, we're following up with the .NET team for guidance on this.

@mattchenderson mattchenderson marked this pull request as draft February 25, 2025 00:38
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.

2 participants