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

Paket template packagetypes #3212

Merged
merged 4 commits into from May 19, 2018

Conversation

@enricosada
Copy link
Collaborator

@enricosada enricosada commented May 18, 2018

add packageTypes to paket.template

used by newer nuspec schema for types like Template, DotnetTool, DotnetCliTool

is optional, and default is not set

@enricosada enricosada requested a review from matthid May 18, 2018
@@ -163,6 +163,8 @@ LANGUAGE
en-gb
tags
rop, fsharp F#
packageTypes
DotnetTool, Template

This comment has been minimized.

@matthid

matthid May 18, 2018
Member

Do we need to normalize user input somehow here?

This comment has been minimized.

@enricosada

enricosada May 18, 2018
Author Collaborator

no, i think is better to leave these as is, as normal string list.
nuspec doesnt show an enumeratin of values, is just like a tag about the package type.
Nuget team may have choosen to just use some tag, but instead is another property :( more complexity, few gains

also like that is less maintenance when new types are added. is really a small use case ihmo (template, clitool, dotnetool). and last two is better to be create with dotnet restore probably

This comment has been minimized.

@matthid

matthid May 18, 2018
Member

Yes I'm fine with that (was basically just asking what you think)

This comment has been minimized.

@enricosada

enricosada May 18, 2018
Author Collaborator

Yes, i was thinking about it too, good question.
And to validate it too (to output the right word with the same case as nuget, to play safe).
but use case is really small

Copy link
Member

@matthid matthid left a comment

Looks reasonable to me

@matthid matthid merged commit 087dda3 into fsprojects:master May 19, 2018
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants