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

Artifacts potentially packs build scripts in wrong folder #145

Closed
chris-sekira opened this issue Jan 17, 2020 · 4 comments
Closed

Artifacts potentially packs build scripts in wrong folder #145

chris-sekira opened this issue Jan 17, 2020 · 4 comments
Labels
Bug Something isn't working

Comments

@chris-sekira
Copy link
Contributor

The Artifacts project has props/targets that get packed into the buildCrossTargeting folder. According to the NuGet package folder layout documentation this should be the buildMultiTargeting instead.

Unless this some kind of a legacy workaround/disambiguation, the folder should probably be renamed.

If I'm wrong on this and there's documentation that you've found for using the buildCrossTargeting, I'd be very appreciated if you could attach a link so that I can bookmark it for myself and request it's addition to the linked documentation above.

@jeffkl jeffkl added the Bug Something isn't working label Jan 17, 2020
@jeffkl
Copy link
Contributor

jeffkl commented Jan 17, 2020

NuGet renamed the folder and supports both at least for a while. We should rename it.

NuGet/Home#4098

@chris-sekira
Copy link
Contributor Author

Ah, google failed me. If buildMultiTargeting is the new standard, I'll look into submitting a change.

The oldest supported framework used here is net46, which was released before the linked issue's decision to move to buildMultiTargeting. Any chance we'd need to support buildCrossTargeting for anyone using older clients?

@jeffkl
Copy link
Contributor

jeffkl commented Jan 17, 2020

I can't think of a reason to keep both folders since people using older clients can just use an older package. I think renaming the folder is the right solution. Please feel free to contribute the change, otherwise I can get to it next week.

@chris-sekira
Copy link
Contributor Author

@jeffkl I'll make the change during lunch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants