Skip to content

Conversation

drewnoakes
Copy link
Member

@drewnoakes drewnoakes commented Oct 17, 2024

Description

Adds two new methods:

  • ResourceExtensions.HasAnnoationOfType<T>
  • ResourceExtensions.HasAnnotationIncludingAncestorsOfType<T>

Also modify TryGetAnnotationOfType and TryGetAnnotationsIncludingAncestorsOfType to reduce allocations and remove stack-based recursion.

Expand test coverage.

Some XML doc tweaks.

Would be helpful in #6209, where TryGet* is used and the out param discarded.

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No
  • Does the change require an update in our Aspire docs?
    • Yes
      • Link to aspire-docs issue:
    • No
Microsoft Reviewers: Open in CodeFlow

Also modify the TryGetAnnotation methods to reduce allocations and remove stack-based recursion.

Expand test coverage.

Some XML doc tweaks.
@drewnoakes drewnoakes added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Oct 17, 2024
@drewnoakes drewnoakes requested a review from adamint October 17, 2024 12:42
@JamesNK
Copy link
Member

JamesNK commented Oct 17, 2024

This is post 9.0? It doesn't seem like an important bug fix.

@JamesNK JamesNK added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Oct 17, 2024
@drewnoakes
Copy link
Member Author

The idea was that the API would be available for @adamint to consume in #6209, but that's merged now anyway.

@drewnoakes drewnoakes removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Nov 11, 2024
@drewnoakes drewnoakes merged commit cc15725 into main Nov 11, 2024
9 checks passed
@drewnoakes drewnoakes deleted the drnoakes/annotation-helper-methods branch November 11, 2024 00:58
@github-actions github-actions bot locked and limited conversation to collaborators Dec 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants