-
Notifications
You must be signed in to change notification settings - Fork 4
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
Create chocolatey package as a part of the build #24
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few comments, but on the whole, looks good to me.
|
||
<title>NUnit Project Editor</title> | ||
<authors>Charlie Poole</authors> | ||
<!-- projectUrl is required for the community feed --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CharliePoole don't think you need this comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deleted
<description>NUnit supports NUnit projects (extension .nunit) to define a collection of test assemblies that are loaded and run together. This tool is used to edit the contents of an NUnit Test Project.</description> | ||
<releaseNotes>This is the first independent production release of the editor, which was previously bundled with NUnit V2.</releaseNotes> | ||
|
||
<!--<provides>NOT YET IMPLEMENTED</provides>--> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CharliePoole you can remove these just now as they are not implemented in chocolatey yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deleted
Task("PackageChocolatey") | ||
.Does(() => | ||
{ | ||
CreateDirectory("PACKAGE_DIR"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CharliePoole would EnsureDirectoryExists be more appropriate here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copied from another task - changed both now.
{ | ||
new ChocolateyNuSpecContent { Source = "LICENSE.txt" }, | ||
new ChocolateyNuSpecContent { Source = "CHANGES.txt" }, | ||
new ChocolateyNuSpecContent { Source = BIN_DIR + "nunit-editor.exe", Target = "bin" }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CharliePoole since you are embedding the exe in the package, which is fine and encouraged, we would ask that you include a verification.txt. This is used by the moderators and community members to ensure that the contained binary is legitimate. Ideally, this would include the checksums for the files. Here is an example: https://github.com/chocolatey/chocolatey-coreteampackages/blob/b3fa04db54fc2b649959ef94faa1c4e704bd75bb/automatic/qbittorrent/tools/VERIFICATION.txt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I understand this. The docs indicate it should be done when the package downloads an installer from a url using http: or https:. This package contains the binaries themselves rather than a setup file.
I suppose one approach would be for you to compare a checksum over the entire nupkg that's uploaded to chocolatey to the same nupkg as downloaded from github or from our myget feed.
Do you have any example of a package that's structured in the same way and how that gets verified?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
VERIFICATION isn't necessary when the package is coming from a trusted source. It's helpful when a 3rd party maintainer is embedding software directly into a package as a means of verifying the original. Some software vendors only release to the Chocolatey community package repository and that is fine. I don't know fully what you folks talked about, so I don't know what outcome was decided. Don't let me confuse anything that was decided on though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leaving it out then.
ProjectSection(SolutionItems) = preProject | ||
build.cake = build.cake | ||
nunit-project-editor.nuspec = nunit-project-editor.nuspec | ||
EndProjectSection |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What benefit is adding these 2 files to the solution? Ease of editing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. :-)
After a skype chat with @gep13 I have a direction to go for the verification of the package. I'll work on that before merging. Other comments have already been reflected in the PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry this is a few days late Charlie. Looks good - looking forward to seeing everything on chocolatey properly! 😄
new ChocolateyNuSpecContent { Source = "LICENSE.txt" }, | ||
new ChocolateyNuSpecContent { Source = "CHANGES.txt" }, | ||
new ChocolateyNuSpecContent { Source = BIN_DIR + "nunit-editor.exe", Target = "bin" }, | ||
new ChocolateyNuSpecContent { Source = BIN_DIR + "nunit.ico", Target = "bin" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this be better coming from nunit/resources
? I think the nuspecs support URLs, would need checking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes they do. It's only that this has been underway since before we got the resources repo set up. I'll update now that we have decided how to use it.
<package xmlns="http://schemas.microsoft.com/packaging/2015/06/nuspec.xsd"> | ||
<metadata> | ||
<id>nunit-project-editor</id> | ||
<version>1.0</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this take a $version
parameter from the cake script? 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can and our nuget packages do. I just need to look up how to do it with choco as opposed to nuget.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CharliePoole @ChrisMaddock this should work in exactly the same way as you do it for NuGet nuspec files. This is how we do it for Cake:
https://github.com/cake-build/cake/blob/develop/build.cake#L231
https://github.com/cake-build/cake/blob/develop/nuspec/Cake.Portable.nuspec#L7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @gep13 !
<metadata> | ||
<id>nunit-project-editor</id> | ||
<version>1.0</version> | ||
<packageSourceUrl>https://github.com/nunit/nunit-project-editor/chocolatey/</packageSourceUrl> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't look like there will be a /chocolatey/
- did things get moved around?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup! The original source created by choco new
was an entire directory with scripts, etc. I have now managed to get rid of all that so we can work on Linux as well. I'll update the url.
<title>NUnit Project Editor</title> | ||
<authors>Charlie Poole</authors> | ||
<projectUrl>https://github.com/nunit/nunit-project-editor</projectUrl> | ||
<iconUrl>http://cdn.rawgit.com/nunit/resources/master/images/icon/nunit_64.png</iconUrl> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We decided over on nunit/nunit3-vs-adapter#268 that we were going to use 256x256 icons for NuGet packages - I presume chocolatey should do the same?
@gep13 - is there a recommended size?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll check further and update if needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ChrisMaddock @CharliePoole the recommendations around icons are here: https://chocolatey.org/docs/create-packages#package-icon-guidelines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on that, I guess we should use 128x128 rather than 256x256 for chocolatey.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think it would save our sanity a little to also use 256x256 for chocolatey packages? It would look better on high-def screens, and save us adding a 4th 'standard size' to keep track of. 😆 The recommendation is also a 'minimum' size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
256x256 is fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ferventcoder I was just away to say the same. There are a number of packages already, for example this one https://chocolatey.org/packages/calibre, that uses 256 x 256.
<projectUrl>https://github.com/nunit/nunit-project-editor</projectUrl> | ||
<iconUrl>http://cdn.rawgit.com/nunit/resources/master/images/icon/nunit_64.png</iconUrl> | ||
<copyright>2016 Charlie Poole</copyright> | ||
<licenseUrl>http://github.com/nunit/nunit-project-editor/LICENSE.txt</licenseUrl> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a dead link for me. Should it be this? http://cdn.rawgit.com/nunit/nunit-project-editor/master/LICENSE.txt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll check. However, I'm not sure we want to use rawgit for the license.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you want https://github.com/nunit/nunit-project-editor/blob/master/LICENSE.txt.
If embedding the software, we also like that license file copied right into the Chocolatey package as well. That way it also covers point in time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed the link.
@ferventcoder It's also in the package.
<licenseUrl>http://github.com/nunit/nunit-project-editor/LICENSE.txt</licenseUrl> | ||
<requireLicenseAcceptance>false</requireLicenseAcceptance> | ||
<projectSourceUrl>https://github.com/nunit/nunit-project-editor</projectSourceUrl> | ||
<docsUrl>https://github.com/nunit/docs/wiki/Project-Editor</docsUrl> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good shout including this! Chocolatey also has a supportUrl
and bugTrackerUrl
- which could be good additions? 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can include them if you think it's helpful (it's optional as far as chocolatey.org is concerned).
Not sure whether to use a mailto: or the google groups url for support. You need to have a google id for the latter, so maybe the email is better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd personally prefer a URL, mailto links are often naff when you use an online email client! That's just a personal preference however. 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Off the top of my head, not sure how a mailto: url would be rendered via the markdown engine, so probably safer to stick with a URL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ChrisMaddock supportUrl
is not accepted. I used mailingListUrl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops sorry - looks like that one's planned but not implemented yet. Can't remember where I found it.
I'll do a few more changes based on the latest round of comments. |
Plan is to merge this and then manually submit it as a pre-release to chocolatey.org. We'll continue manually for a bit and eventually start pushing automatically. |
e6ff646
to
b7ea244
Compare
I don't know... I just did it the way @gep13 recommended.
…On Sat, Dec 31, 2016 at 5:36 AM, Chris Maddock ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In nunit-project-editor.nuspec
<#24 (review)>
:
> @@ -0,0 +1,25 @@
+<!-- Do not remove this test for UTF-8: if “Ω” doesn’t appear as greek uppercase omega letter enclosed in quotation marks, you should use an editor that supports UTF-8, not this one. -->
+<package xmlns="http://schemas.microsoft.com/packaging/2015/06/nuspec.xsd">
+ <metadata>
+ <id>nunit-project-editor</id>
+ <version>0.0.0</version>
Should this use $version$ like our other nuspecs, to force the value to
be passed as a parameter, or did that not work for chocolatey?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#24 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACjfha73I4tiDLVaFGI-R05jvbVCzWevks5rNlpagaJpZM4LVi9I>
.
|
Chocolatey supports passing a version to choco pack. |
If calling Hope that helps! |
Whatever you decide is best, Cake does a great job of handling this so if that is what you are using to build then I would say use it. |
Interesting! I didn't know that, I had assumed it was the same as nuget. Shame on me! |
Fixes #21
I tested the install and uninstall of the package locally. One problem with initial development is that there seems to be no way to see how it will look in the chocolatey.org site once uploaded. This is of course a common problem to nuget.org and the vs gallery as well so I guess we will get around it.
@ferventcoder I'd appreciate your taking a look at this along with any @nunit/core-team members.