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

Further net - pullrequest tuning #48379

Merged
merged 11 commits into from
Feb 26, 2025
Merged

Further net - pullrequest tuning #48379

merged 11 commits into from
Feb 26, 2025

Conversation

scbedd
Copy link
Member

@scbedd scbedd commented Feb 21, 2025

This PR:

  • Removes the isOther function in language-settings as the built-in functionality in Get-PrPkgProperties now resolves for Shared packages if the necessary metadata is present.
  • Ensures that we don't include all service.project files when a overridefile is presented (which was what was causing azure.core tests to show up everywhere)
  • Removes the "add all packages for service directory if *.Shared-named package is present" in favor of a explicit usage of triggeringPaths. Now if Azure.Messaging.EventHubs.Shared is changed, we will indirectly run tests for Azure.Messaging.EventHubs and Azure.Messaging.EventHubs.Processor, but NONE of the other track1 / functions packages. @jsquire FYI
  • Make IntegrationTests a class of package that can be directly excluded from property. Exclude integration test packages from pack for PRs.

todo:

  • Walk all incidences Shared packages like for eventhub to ensure their triggeringPaths are configured properly.
  • Understand why we are so aggressively adding Microsoft. packages when we've explicitly updated the language-settings to not cause that anymore.
    • Need changes to ci.yml files must only apply to the artifacts within. (this is why the microsoft eventhub namespaces are being added rn)

@scbedd scbedd requested a review from a team as a code owner February 22, 2025 01:21
…e that we don't attempt to build the .net integration tests when packing
@scbedd
Copy link
Member Author

scbedd commented Feb 25, 2025

@weshaggard ok I think this is ready to go in. There is another cleanup commit I could do re: removing the triggering paths for template, but honestly I don't want to add another commit to this PR. There will be one last follow-up where I figure out what I want to do with service-level changes to directories with multiple ci.yml files, and I'll clean up the triggering paths there.

@scbedd
Copy link
Member Author

scbedd commented Feb 26, 2025

/check-enforcer override

The snippets issue in analyze for keyvault is present in main, so ignoring on this PR.

@scbedd scbedd merged commit bb5efb6 into main Feb 26, 2025
139 of 141 checks passed
@scbedd scbedd deleted the further-pr-tuning branch February 26, 2025 00:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🎊 Closed
Development

Successfully merging this pull request may close these issues.

2 participants