(7zip*) migrated from ferventcoder #488

majkinetor · 2016-12-21T09:10:20Z

Package source: (7zip*) migrated to coreteam repository ferventcoder/chocolatey-packages#223
3 open issues

pascalberger · 2016-12-22T13:04:33Z

@majkinetor Will you change this also to an embedded package? Based on the license I think this should be possible.

majkinetor · 2016-12-22T13:31:30Z

Will you change this also to an embedded package?

Yes, this should be embedded. You, rather then me, should do it, since you are migrating it now. Its easier then creating online installer and people should get used to it. See vlc as an example or any other that uses it.

In short:

you can use Get-RemoteFiles in this repository (still unpublished) but this repo uses NEXT version.
you will need to create embedded intaller which differes from online one (no checksums, urls etc.). I created Get-RemoteFiles in such way that changes to chocolateyInstall are trivial (in general case only silentArgs should be filled in and removed arch's that aren't used).

majkinetor · 2016-12-22T13:31:55Z

Ah, sorry, I thought I was writting on Git PR.

Yes, I will embedd this one.

gep13 · 2016-12-24T09:54:44Z

@majkinetor chocolatey user has been added to each of these packages on chocolatey.org.

ferventcoder · 2017-01-12T03:56:28Z

This ready?

AdmiringWorm · 2017-01-12T03:57:55Z

@ferventcoder doesn't look like it, looks like it's currently just the same code as in your repo

EDIT:
I plan to continue on @majkinetor work this wekend, unless he finds the time to do it instead.

ferventcoder · 2017-01-12T15:04:09Z

Right on.

fixes ferventcoder/chocolatey-packages#198

fixes ferventcoder/chocolatey-packages#188

AdmiringWorm · 2017-01-14T17:59:12Z

I believe this is done now, could someone please review it?

All issues at @ferventcoder is fixed except for ferventcoder/chocolatey-packages#82 which I couldn't reproduce, making me think it have been fixed upstream

pascalberger · 2017-01-14T19:36:03Z

automatic/7zip.commandline/7zip.commandline.nuspec

+    <title>7-Zip (Portable, CommandLine)</title>
+    <version>0.0</version>
+    <authors>Igor Pavlov</authors>
+    <owners>Rob Reynolds, chocolatey</owners>


List chocolatey first as per §1.2.4

ah thanks, I forgot about that one.
Don't know why it's in there though, as the owners element isn't used on chocolatey.org as far as I know.

pascalberger · 2017-01-14T19:37:33Z

automatic/7zip.commandline/7zip.commandline.nuspec

+    <docsUrl>http://www.7-zip.org/faq.html</docsUrl>
+    <mailingListUrl>https://sourceforge.net/p/sevenzip/discussion/45797/</mailingListUrl>
+    <bugTrackerUrl>https://sourceforge.net/p/sevenzip/_list/tickets?source=navbar</bugTrackerUrl>
+  </metadata>


Add <releaseNotes> (http://www.7-zip.org/history.txt)

thanks, I wasn't able to find the changelog for 7zip

pascalberger · 2017-01-14T19:39:09Z

automatic/7zip.commandline/7zip.commandline.nuspec

+<?xml version="1.0" encoding="utf-8"?>
+<package xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
+  <metadata>
+    <id>7zip.commandline</id>


Shouldn't we take the oppertunity and rename this to 7zip.portable?

sure, we can do that.
I just didn't have that as a priority at the moment

pascalberger · 2017-01-14T19:41:22Z

automatic/7zip.commandline/tools/chocolateyInstall.ps1

+$packageArgs.file        = $filePathExtra
+Get-ChocolateyUnzip @packageArgs
+
+Remove-Item -Path "$toolsDir\Uninstall.exe",$filePath32,$filePath64,$filePathExtra -Force


Add -ea 0 to avoid install failing in case this fails, like it was done in other packages

pascalberger · 2017-01-14T19:42:46Z

automatic/7zip.commandline/update.ps1

+  $Latest.ChecksumType = 'sha256'
+
+  try {
+    $client = New-Object System.Net.WebClient


Should be outside of the try, since in case this throws an exception you don't want the finally block to be run, since it would throw another exception.

this isn't .NET, so it don't matter if it's outside or inside the try/catch. (as far as I know)

but I'll change it anyhow

pascalberger · 2017-01-14T19:48:36Z

automatic/7zip.install/tools/chocolateyInstall.ps1

+  Start-Process explorer.exe
+}
+
+Remove-Item "$filePath32*","$filePath64*" -Force -ErrorAction SilentlyContinue


-ea 0 instead of -ErrorAction SilentlyContinue to be consistent with other packages

pascalberger · 2017-01-14T19:51:46Z

automatic/7zip.install/update.ps1

+function global:au_SearchReplace {
+  @{
+    ".\tools\chocolateyInstall.ps1" = @{
+      "(?i)(^\s*softwareName\s*=\s*)'.*'" = "`$1'$softwareNamePrefix $($Latest.RemoteVersion)*'"


IMHO using $Latest.RemoteVersion is not 100% acurrate. It makes sense as long as the already installed version is the previous version, but what if I skip versions? But currently also don't have a better idea :(

Actually, in this case it will be accurate.
The software name should be part of or equal to the name displayed in program and features

Minus the version IMHO as well. If need be the major version.

pascalberger · 2017-01-14T19:53:01Z

automatic/7zip.install/update.ps1

+      "(?i)(^\s*softwareName\s*=\s*)'.*'" = "`$1'$softwareNamePrefix $($Latest.RemoteVersion)*'"
+    }
+    ".\tools\chocolateyUninstall.ps1" = @{
+      "(?i)(\s*\-SoftwareName\s+)'.*'" = "`$1'$softwareNamePrefix $($Latest.RemoteVersion)*'"


Shouldn't it be $Latest.Version here?

no, $Latest.Version will be changed if there is a fix version, which will make the uninstall script unusable since it won't be able to find the uninstall key

Why the version here?

pascalberger · 2017-01-14T19:53:28Z

automatic/7zip/7zip.nuspec

+    <title>7-Zip</title>
+    <version>0.0</version>
+    <authors>Igor Pavlov</authors>
+    <owners>Rob Reynolds, chocolatey</owners>


List chocolatey first as per §1.2.4

pascalberger · 2017-01-14T19:54:16Z

automatic/7zip/7zip.nuspec

+    <iconUrl>https://cdn.rawgit.com/chocolatey/chocolatey-coreteampackages/dbedb56d5ff709cd37f4abecc736d4a9e6400da8/icons/7zip.svg</iconUrl>
+    <docsUrl>http://www.7-zip.org/faq.html</docsUrl>
+    <mailingListUrl>https://sourceforge.net/p/sevenzip/discussion/45797/</mailingListUrl>
+    <bugTrackerUrl>https://sourceforge.net/p/sevenzip/_list/tickets?source=navbar</bugTrackerUrl>


Add <releaseNotes> (http://www.7-zip.org/history.txt)

although it doesn't show up, I've added the release notes below this line

@AdmiringWorm Where and why doesn't it show up? Should be shown on chocolatey.org...

sorry, I meant it doesn't show where just commented as it was added on the line below

Ah, comments in GitHub can be confusing :) I think I commented the closing metadata tag

you did for the other two packages.

AdmiringWorm · 2017-01-14T20:52:30Z

@pascalberger
I believe I've added/responded to the feedback you provided.
With the exception of moving to 7zip.portable, will do that to but not yet.

AdmiringWorm · 2017-01-14T21:12:13Z

I believe the 7zip packages is now finally done.

/cc @pascalberger @ferventcoder

pascalberger · 2017-01-14T21:31:41Z

Thanks, LGTM

AdmiringWorm · 2017-01-14T21:36:28Z

I'm just looking for a 👍 from @ferventcoder before this can be merged.

AdmiringWorm · 2017-01-17T04:24:19Z

I noticed the following packages need to be unrejected.
7zip v16.04
7zip.install v16.04

Not sure about 7zip.commandline v16.04 needs to unrejected though, as we're deprecating it.

/cc @chocolatey/community-moderators

gep13 · 2017-01-17T13:16:07Z

@AdmiringWorm I have unrejected the first two packages. For the last one, I would be tempted to push out a package fix notation version, that way it is clear that it isn't an "official" release.

ferventcoder · 2017-01-18T06:26:51Z

automatic/7zip.install/tools/chocolateyUninstall.ps1

+$packageName = '7zip.install'
+
+$uninstalled = $false
+[array]$key = Get-UninstallRegistryKey -SoftwareName '7-zip 16.04*'


Never a fan of passing the version. It doesn't upgrade well.

Originally my reason for passing the version was to allow users to install multiple 7zip versions side-by-side, but noticed now that it doesn't matter since the registry key is overwritten anyhow.
I'll change it back to not pass the version.

Version 9.x and 15/16.x can be installed separately.

ferventcoder · 2017-01-18T06:37:28Z

deprecated/packages/7zip.commandline/7zip.commandline.nuspec

+<package xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
+  <metadata>
+    <id>7zip.commandline</id>
+    <title>[Deprecated]7-Zip (Portable, CommandLine)</title>


Can we get a space after "]"? It should always be "[Deprecated] Title"

ferventcoder · 2017-01-18T06:38:20Z

Looks great. Just a few minor issues...

AdmiringWorm · 2017-01-18T07:05:35Z

@ferventcoder I've made the requested changes, if there isn't anything else I believe this can now be merged.

ferventcoder · 2017-01-18T07:10:55Z

LGTM

AdmiringWorm · 2017-01-18T07:14:32Z

👍
The build is running to update the 7zip packages now, when it's done and the 7zip.portable package is approved I can push out the deprecated 7zip.commandline package

(7zip*) original ferventcoder data

dbedb56

majkinetor added the 2 - Working label Dec 21, 2016

majkinetor self-assigned this Dec 21, 2016

majkinetor changed the title ~~(7zip*) migrated from ferventcoder~~ (7zip*) migrated from ferventcoder [Requires chocolatey user] Dec 23, 2016

pascalberger changed the title ~~(7zip*) migrated from ferventcoder [Requires chocolatey user]~~ (7zip*) migrated from ferventcoder Dec 24, 2016

majkinetor removed their assignment Dec 25, 2016

majkinetor removed the 2 - Working label Jan 12, 2017

AdmiringWorm self-assigned this Jan 14, 2017

AdmiringWorm added the 2 - Working label Jan 14, 2017

AdmiringWorm added 12 commits January 14, 2017 18:49

(7zip) Removed ketarin file

a271934

(7zip) Added AU update script

1174267

(7zip) Updated metadata

b8d3e2b

(7zip.install) Updated metadata

c633732

(7zip.install) Updated install script and embedded installer

fe4c5a4

fixes ferventcoder/chocolatey-packages#198

(7zip.install) Added AU update script

e66c143

(7zip) Added uninstall script

798cb01

fixes ferventcoder/chocolatey-packages#188

(7zip.install) Added legal notes

bb5fd6c

(7zip.commandline) Updated metadata

7940972

(7zip.commandline) Added legal notes

17e2b62

(7zip.commandline) Embedded package

27c7455

(7zip.commandline) Added AU update script

6621cc0

AdmiringWorm added 4 - Done and removed 2 - Working labels Jan 14, 2017

AdmiringWorm requested review from ferventcoder, gep13 and pascalberger January 14, 2017 17:57

pascalberger requested changes Jan 14, 2017

View reviewed changes

AdmiringWorm added 4 commits January 14, 2017 21:43

(7zip*) Added release notes + reordered owners

b982905

(7zip*) Changed Remove-Item to be the same as other packages

bc5193d

(7zip.install) Extracted function to helpers.ps1

a2643d7

(7zip.commandline) Moved variable to outside of try/catch block

eec0f5a

AdmiringWorm added 2 commits January 14, 2017 22:09

(7zip.commandline) Renamed to 7zip.portable

c6a31e9

(7zip.commandline) Added as a deprecated package

85b0a46

pascalberger approved these changes Jan 14, 2017

View reviewed changes

ferventcoder reviewed Jan 18, 2017

View reviewed changes

AdmiringWorm added 2 commits January 18, 2017 08:01

(7zip.install) Changed to not pass version as softwareName

90407d0

(7zip.commandline) Added space between Deprecated] and 7-Zip

ada3ad4

AdmiringWorm merged commit 68b91a8 into master Jan 18, 2017

AdmiringWorm deleted the 7zip branch January 18, 2017 07:12

(7zip*) migrated from ferventcoder #488

(7zip*) migrated from ferventcoder #488

Conversation

majkinetor commented Dec 21, 2016 • edited Loading

pascalberger commented Dec 22, 2016

majkinetor commented Dec 22, 2016

majkinetor commented Dec 22, 2016 • edited Loading

gep13 commented Dec 24, 2016

ferventcoder commented Jan 12, 2017

AdmiringWorm commented Jan 12, 2017 • edited Loading

ferventcoder commented Jan 12, 2017

AdmiringWorm commented Jan 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AdmiringWorm commented Jan 14, 2017

AdmiringWorm commented Jan 14, 2017

pascalberger commented Jan 14, 2017

AdmiringWorm commented Jan 14, 2017

AdmiringWorm commented Jan 17, 2017

gep13 commented Jan 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ferventcoder commented Jan 18, 2017

AdmiringWorm commented Jan 18, 2017

ferventcoder commented Jan 18, 2017

AdmiringWorm commented Jan 18, 2017

majkinetor commented Dec 21, 2016 •

edited

Loading

majkinetor commented Dec 22, 2016 •

edited

Loading

AdmiringWorm commented Jan 12, 2017 •

edited

Loading