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

[Neo Plugin Fix] Fix targes #3343

Closed
wants to merge 1 commit into from
Closed

Conversation

Jim8y
Copy link
Contributor

@Jim8y Jim8y commented Jun 17, 2024

Description

This pr fixes the issue of multiple targes for plugins while there is only one target specified. Making it impossible for dotnet publish to automatically detect the target.

Fixes # #3344

Type of change

  • Optimization (the change is only an optimization)
  • Style (the change is only a code style for better maintenance or standard purpose)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Test Configuration:

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@Jim8y Jim8y requested a review from cschuchardt88 June 17, 2024 08:24
@Jim8y Jim8y linked an issue Jun 17, 2024 that may be closed by this pull request
cschuchardt88
cschuchardt88 previously approved these changes Jun 17, 2024
Copy link
Member

@cschuchardt88 cschuchardt88 left a comment

Choose a reason for hiding this comment

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

Whats the problem with the Targets?

Reason for so we can do Standard or any other target in feature. Unless you have a problem coming up, than I dont see the point.

@cschuchardt88 cschuchardt88 dismissed their stale review June 17, 2024 09:53

This PR breaks github actions and building environment.

@cschuchardt88
Copy link
Member

This was fixed already

image

@cschuchardt88
Copy link
Member

cschuchardt88 commented Jun 17, 2024

Why you approving something is breaks our environment. everyone's inexperienced or lack of knowledge of dotnet is disappointing when the issue is just that.

This break tests and other things in your our github workflows. Including the environment locally.

Copy link
Member

@cschuchardt88 cschuchardt88 left a comment

Choose a reason for hiding this comment

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

Doesnt work in all enviroments, breaks testing and github workflows, along with local development and other frameworks like dotnet standard.

@Jim8y
Copy link
Contributor Author

Jim8y commented Jun 17, 2024

This was fixed already

image

i know we can do this, but why we need to do this while there is only one target supported? why use targets instead of target? and why this can cause problem? BTW, dont worry about approvals, as long as workflow fails, pr can not be merged.

@cschuchardt88
Copy link
Member

cschuchardt88 commented Jun 17, 2024

Because we build for dotnet Standard too from solution or *.csproj that is the reason, or it will fail on other OSS like this PR did. So you need TargetFrameworks not TargetFramework.

@Jim8y
Copy link
Contributor Author

Jim8y commented Jun 18, 2024

Close as #3343 (comment)

@Jim8y Jim8y closed this Jun 18, 2024
@Jim8y Jim8y deleted the fix-publish branch June 18, 2024 01:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dotnet publish doesn't work in plugin's project path
4 participants