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

fix: default downloaded file name #7597

Merged
merged 3 commits into from
Jun 16, 2023

Conversation

@changeset-bot
Copy link

changeset-bot bot commented Jun 2, 2023

🦋 Changeset detected

Latest commit: 7097f3e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
electron-updater Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@netlify
Copy link

netlify bot commented Jun 2, 2023

Deploy Preview for car-park-attendant-cleat-11576 ready!

Name Link
🔨 Latest commit 7097f3e
🔍 Latest deploy log https://app.netlify.com/sites/car-park-attendant-cleat-11576/deploys/6479ba9351f6b70008b5fe44
😎 Deploy Preview https://deploy-preview-7597--car-park-attendant-cleat-11576.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@mmaietta
Copy link
Collaborator

mmaietta commented Jun 4, 2023

Just to double-check. How have you been testing/deploying this change locally?

I'm trying to understand what this change is doing here. Why can't it be update.AppImage? This change is affecting all downloaded updates

For AppImages, it's already supposed to be reusing the original name of the installed AppImage.

const existingBaseName = path.basename(appImageFile)
// https://github.com/electron-userland/electron-builder/issues/2964
// if no version in existing file name, it means that user wants to preserve current custom name
if (path.basename(options.installerPath) === existingBaseName || !/\d+\.\d+\.\d+/.test(existingBaseName)) {
// no version in the file name, overwrite existing
destination = appImageFile
} else {
destination = path.join(path.dirname(appImageFile), path.basename(options.installerPath))
}

@marcuskirsch
Copy link
Contributor Author

marcuskirsch commented Jun 5, 2023

Just to double-check. How have you been testing/deploying this change locally?

I'm trying to understand what this change is doing here. Why can't it be update.AppImage? This change is affecting all downloaded updates

For AppImages, it's already supposed to be reusing the original name of the installed AppImage.

const existingBaseName = path.basename(appImageFile)
// https://github.com/electron-userland/electron-builder/issues/2964
// if no version in existing file name, it means that user wants to preserve current custom name
if (path.basename(options.installerPath) === existingBaseName || !/\d+\.\d+\.\d+/.test(existingBaseName)) {
// no version in the file name, overwrite existing
destination = appImageFile
} else {
destination = path.join(path.dirname(appImageFile), path.basename(options.installerPath))
}

I have not seen this part of the code. But you should call the AppImage not update.AppImage but like it is in the latest-linux.yml? When I do an update, for example, XY-1.4.9.AppImage is renamed to update.AppImage.After that, no one knows what is behind the AppImage.

I tested this locally for Linux.

@mmaietta
Copy link
Collaborator

I like this change! 👍

Just to confirm, have you tested this on Mac + Windows?

@marcuskirsch
Copy link
Contributor Author

marcuskirsch commented Jun 14, 2023

@mmaietta Unfortunately, I don't have the possibility to test it in other environments. Could you? Pleeaaaase…

@marcuskirsch
Copy link
Contributor Author

@mmaietta i tested it in some virtual machines. Everything works like expected. I guess you can approve that little puppy. ;)

@mmaietta mmaietta merged commit cd15e16 into electron-userland:master Jun 16, 2023
13 checks passed
@github-actions github-actions bot mentioned this pull request Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants