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

Add ExcludeLaunchProfile method to model #1978

Merged
merged 7 commits into from
Feb 5, 2024
Merged

Conversation

JamesNK
Copy link
Member

@JamesNK JamesNK commented Jan 30, 2024

Fixes #1849

There appears to be two ways to launch a project:

  • Via IDE
  • Via dotnet watch

ExcludeLaunchProfile has been added to dotnet watch path by skipping reading launch profile.

I'm guessing ExcludeLaunchProfile needs to be passed to the IDE as an annotation. Does one exist today? What should the name/value be?

Microsoft Reviewers: Open in CodeFlow

@JamesNK JamesNK added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Jan 30, 2024
Copy link
Member

@mitchdenny mitchdenny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@mitchdenny mitchdenny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resetting vote because I realized there is still some conversation pending around how suppress launch profile on the DCP side.

@JamesNK
Copy link
Member Author

JamesNK commented Jan 31, 2024

Blocked on DCP changes.

@JamesNK
Copy link
Member Author

JamesNK commented Jan 31, 2024

Feedback applied. @karolz-ms Can you double check the usage of the new annotation. Feels pretty simple.

Can this change be merged now or does it need to wait for the annotation to be supported in DCP?

@JamesNK JamesNK removed the blocked label Jan 31, 2024
Copy link
Member

@karolz-ms karolz-ms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great

@karolz-ms
Copy link
Member

Can this change be merged now or does it need to wait for the annotation to be supported in DCP?

We can merge now. The setting will not be effective until DCP and VS pieces are in place, but it won't do any harm.

Copy link
Member

@mitchdenny mitchdenny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed a merge conflict.

@JamesNK JamesNK force-pushed the jamesnk/excludelaunchprofile branch from e3884e5 to 8873c1f Compare February 5, 2024 03:02
@JamesNK JamesNK enabled auto-merge (squash) February 5, 2024 03:04
@JamesNK JamesNK merged commit 774d541 into main Feb 5, 2024
8 checks passed
@JamesNK JamesNK deleted the jamesnk/excludelaunchprofile branch February 5, 2024 03:40
radical pushed a commit to radical/aspire that referenced this pull request Feb 6, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Apr 22, 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.

Add support for ignoring launch profiles for project resources
4 participants