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.

Sign up for GitHub

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

Jump to bottom

KDE.Dolphin.Release #33718

Closed
wants to merge 2 commits into from
Closed

KDE.Dolphin.Release #33718

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
066d802
Imoprovement.
Nov 3, 2021
5c221a7
Specification.
Nov 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions manifests/k/KDE/Dolphin/21.04.3/KDE.Dolphin.installer.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
# Created using wingetcreate 0.4.1.1
# yaml-language-server: $schema=https://aka.ms/winget-manifest.installer.1.0.0.schema.json

PackageIdentifier: KDE.Dolphin
PackageVersion: 21.04.3
PackageIdentifier: KDE.Dolphin.Release
Copy link
Contributor

@ItzLevvie ItzLevvie Nov 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
PackageIdentifier: KDE.Dolphin.Release
PackageIdentifier: KDE.Dolphin

I believe this isn't needed here since this would likely cause inconsistencies with other applications outside of what KDE publishes.

PackageVersion: master-1255
Copy link
Contributor

@ItzLevvie ItzLevvie Nov 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
PackageVersion: master-1255
PackageVersion: 21.04.3

PackageVersion must match Control Panel.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/microsoft/winget-pkgs/pull/33718/files/5c221a74c3d79381ea178f4bbc75b7d09b24d70c#r741996478.

Please do observe http://github.com/microsoft/winget-pkgs/pull/33717#issuecomment-959212573. Consequently, is the reversion that you have proposed definitely mandatory?

Scope: machine
InstallModes:
- interactive
- silent
Installers:
- Architecture: x64
InstallerType: nullsoft
InstallerUrl: https://binary-factory.kde.org/job/Dolphin_Release_win64/1255/artifact/dolphin-21.04.3-1255-windows-msvc2019_64-cl.exe
InstallerUrl: https://binary-factory.kde.org/view/Windows%2064-bit/job/Dolphin_Release_win64/lastSuccessfulBuild/artifact/dolphin-21.04.3-1255-windows-msvc2019_64-cl.exe
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall mere HTTP be better than HTTPS?

Copy link
Contributor

@ItzLevvie ItzLevvie Nov 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HTTPS is always preferred over HTTP.

If the URL doesn't work with HTTPS (i.e. server isn't configured to be used for HTTPS), then you can use HTTP.

But this domain supports HTTPS.

Copy link
Contributor

@ItzLevvie ItzLevvie Nov 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
InstallerUrl: https://binary-factory.kde.org/view/Windows%2064-bit/job/Dolphin_Release_win64/lastSuccessfulBuild/artifact/dolphin-21.04.3-1255-windows-msvc2019_64-cl.exe
InstallerUrl: https://binary-factory.kde.org/view/Windows%2064-bit/job/Dolphin_Release_win64/1255/artifact/dolphin-21.04.3-1255-windows-msvc2019_64-cl.exe

Why was this changed? lastSuccessfulBuild in the URL is only valid for 1 day (24 hours), compared to having the build number in the URL which would last for about 4 or 5 days, before needing to be replaced.

If someone doesn't regularly update this like KDE Connect, people will be unable to download due to the expired URL. There will also likely be issue spam from @wingetbot when the URLs are unavailable if it's not updated in a timely matter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

http://github.com/microsoft/winget-pkgs/pull/33718/files/5c221a74c3d79381ea178f4bbc75b7d09b24d70c#r741998016.

The reason that I have specified "/lastSuccessfulBuild/" is because if "/lastSuccessfulBuild/" is used, the logic that is required for automatic replacement by new versions is much more simple, because the name of the version is part of the filename, so also switching directories is quite silly.

Although I did not know that the time that shall be was available for before replacement is mandatory is significantly less than the counterpart of it that you have specified, the automation that is currently present within this repository should remediate that problem.

InstallerSha256: C5FE5779291D5093819AE3153AC0254426F4EFCF188CC8FB32FA73E614F58933
ManifestType: installer
ManifestVersion: 1.0.0
Expand Down