Skip to content

Change property used for after-publish target#47068

Merged
Forgind merged 2 commits intodotnet:release/9.0.3xxfrom
Forgind:change-after-publish-target
Feb 28, 2025
Merged

Change property used for after-publish target#47068
Forgind merged 2 commits intodotnet:release/9.0.3xxfrom
Forgind:change-after-publish-target

Conversation

@Forgind
Copy link
Copy Markdown
Contributor

@Forgind Forgind commented Feb 24, 2025

This is based on feedback here and here.

It seems that the property used for this for web-based projects is typically set in a target, which means it isn't available at evaulation time. This changes the property used to determine if a project is web-based to one set by importing the web sdk.

Although I think it should not be used in almost any case, it also adds the option of specifying that the target should run after a specified target. If a new project type comes up that has an AfterAfterPublish, for instance, and we want to run this after that, this makes it easy to support that.

Copilot AI review requested due to automatic review settings February 24, 2025 18:24
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

@ghost ghost added the untriaged Request triage from a team member label Feb 24, 2025
@ghost
Copy link
Copy Markdown

ghost commented Feb 24, 2025

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

1 similar comment
@ghost
Copy link
Copy Markdown

ghost commented Feb 24, 2025

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@dsplaisted
Copy link
Copy Markdown
Member

We should add a test to cover the case that was broken, IE when using a project that uses the Web SDK.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not too familiar with this and sorry if I'm wrong, but does this properly test whether the target happens after Publish for an SDK project but after AfterPublish for Web Projects? For both project types it seems to only check whether it was after the after publish, which if that was the case, the not UsingMicrosoftNETSdkWeb version could be incorrect?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if I'm interpreting your question correctly, but the relevant difference between these two scenarios is whether a particular target is defined at all, so if this target executes, then we know the target it was 'after' must have executed as well. I verified that one case was a Microsoft.NET.Sdk.Web project, and the other was a Microsoft.NET.Sdk project, so since those are the two scenarios we know are different, this should cover all scenarios, as far as we know. The exact target ordering we leave to MSBuild to get right.

Did that answer your question?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That makes sense, thank you.

@Forgind Forgind merged commit c5d9b7d into dotnet:release/9.0.3xx Feb 28, 2025
30 checks passed
@Forgind Forgind deleted the change-after-publish-target branch February 28, 2025 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

untriaged Request triage from a team member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants