digitalcoyote / chocolatey-packages

Template repository for Chocolatey Automatic Package Updater Module
MIT License
8 stars 11 forks source link

n3dr #4

Closed 030 closed 4 years ago

digitalcoyote commented 4 years ago

Relating to https://github.com/030/n3dr/issues/125 I assume? I'll take a look ASAP. I'm working from a chair with a laptop in an empty room (getting ready to move has taken a toll on my productivity).

EDIT: I forgot to mention this. The easiest way for this repo to automatically update chocolatey is to just add the windows binary to the release on github. Assuming it just needs to be in the PATH to work, this should be pretty simple. If it needs an installer, I can build one with Wix# but Chocolatey takes care of that for a lot of people.

030 commented 4 years ago

@digitalcoyote I have created a PR

030 commented 4 years ago

Please let me know if the Windows binary is fine. The publication was broken, but now it works again. I do not have a windows system. I hope that the format is right. I duplicated the dip that you created.

digitalcoyote commented 4 years ago

I have a VM I run windows tests on :). I'm out at the moment, but I'll be able to take a look at it in a couple hours.

digitalcoyote commented 4 years ago

https://chocolatey.org/packages/n3dr awaiting automated tests

TheCakeIsNaOH commented 4 years ago

Is there any chance this could be moved to an internal package with the binary included in the package rather than downloaded at runtime? Since n3dr is well under 200mb and is under a license that allows redistribution.

I would be willing to provide a PR including AU changes if desired.

digitalcoyote commented 4 years ago

That will be partially up to @030, but if I remember correctly that's not considered a best practice. I'll review the documentation for embedded binaries in packages when the binary is publicly available. I believe that since this would enlarge the size of the package, it could be slower than downloading as part of the install since chocolatey throttles free users to a degree (I'll have to recheck the documentation to make sure this is correct).

TheCakeIsNaOH commented 4 years ago

I believe it is best practice to include binaries, assuming that it is under 200mb and has a license that allows redistribution. This is from the _TODO.txt if you run choco new.

Community Repository? Have Distribution Rights? If you are the software vendor OR the software EXPLICITLY allows redistribution and the total nupkg size will be under 200MB, you have the option to embed the binaries directly into the package to provide the most reliable install experience. Put the binaries directly into the tools folder, use $fileLocation ($file/ $file64) and Install-ChocolateyInstallPackage/ Get-ChocolateyUnzip in tools\chocolateyInstall.ps1. Additionally, fill out the LICENSE and VERIFICATION file (see 3 below and those files for specifics).

Chocolatey does not throttle free users any more than paid users as far as I am aware. Paid users do have the CDN cache, but that is not for the .nupkg files, it is only for files that are downloaded separately during install.

digitalcoyote commented 4 years ago

The "rate limiting" I was concerned with appears to be only for requests, so the size won't matter. I double-checked the license on n3dr and it should be no issue to embed it.

You can submit a PR or I'll try to get to this sometime in the next week or so. Unless @030 objects, I have no valid objections.

030 commented 4 years ago

@digitalcoyote It is fine to include the n3dr binary the package

digitalcoyote commented 4 years ago

PR has been merged. It won't take effect until the next release though. If it's necessary for someone that this release is embedded, I can force an update before it's reviewed or I can force an update post-review.

030 commented 4 years ago

3.6.2 has been released

TheCakeIsNaOH commented 4 years ago

If it's necessary for someone that this release is embedded, I can force an update before it's reviewed or I can force an update post-review.

Don't worry about it, it's not that necessary. And if it was, I could always internalize it manually.

Thanks for the quick review and merge.