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

Migrate to Velopack & Github Action #2616

Open
wants to merge 87 commits into
base: dev
Choose a base branch
from
Open

Migrate to Velopack & Github Action #2616

wants to merge 87 commits into from

Conversation

taooceros
Copy link
Member

@taooceros taooceros commented Mar 24, 2024

Velopack (https://velopack.io/) is the successor of Cloud.Squirrel (https://github.com/clowd/Clowd.Squirrel)

Several change in this pr

  1. Remove shortcut related function https://docs.velopack.io/integrating/shortcuts
  2. Implement a custom restart script which doesn't depend on the updater (which should not be the use case anyway). This allows restart be called at dev environment (breaking change).
  3. Create the portable data folder if the package is not inside %LocalAppData% as velopack will pack the portable version for us, which doesn't allow us to modify (easily) to insert a custom folder.
  4. The post_build script is modified to add information about channel. Prerelease repo will have win-x64-prerelease channel, while the main repo will have win-x64-stable channel. https://docs.velopack.io/packaging/channels
  5. It also contains an experimental github action CI, which has a faster build time and larger concurrency allowance.

TODO:

  • Toggling out of portable via settings doesnt seem to do anything except creating the UserData folder in roaming.
  • Expecting to move portable UserData to roaming, plus start menu and uninstall program entries created.
  • Can we please make the parent folder name FlowLauncher not flowlauncher, same goes for the UserData folder name in roaming.
  • Can we please change the publisher in uninstall program to 'Flow Launcher Team' not 'FlowLauncher'.

Tested:

  • The build file size should be similar size
  • The protable file should create UserData folder
  • Things should work

Futher Consideration:

  • Toggling into portable mode does not remove the uninstall entry in 'Uninstall Program' in control panel, nor does it remove the entry from start menu. (This is moved to Future because there's no easy way to do it except implementing it ourselves).
  • Cleanup post_build script. Build Portable is no longer needed as that will be handled by vpk pack
  • Implement the logic of switching channel (maybe can delay to future). With that and once this pr is merged, we can officially test whether the upgrade process work. I will also create one pr stemmed from master to see how the upgrade from squirrel will behave.

Note: Maybe we want to squash so revert is easier?

@taooceros taooceros changed the title Velopack Migrate to Velopack Mar 24, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@prlabeler prlabeler bot added bug Something isn't working Code Refactor enhancement New feature or request labels Jun 19, 2024
# Conflicts:
#	Flow.Launcher.Core/Flow.Launcher.Core.csproj
#	Flow.Launcher/Flow.Launcher.csproj

This comment has been minimized.

@taooceros
Copy link
Member Author

taooceros commented Jun 19, 2024

@coderabbitai review

@taooceros
Copy link
Member Author

Can we change this two back to their original name please: image

image

This is intentional to use the Release Channel Feature. https://docs.velopack.io/packaging/channels

This comment has been minimized.

This comment has been minimized.

@jjw24
Copy link
Member

jjw24 commented Jun 21, 2024

Can we change this two back to their original name please: image
image

This is intentional to use the Release Channel Feature. https://docs.velopack.io/packaging/channels

This is fine, but you can rename the two files to their proper name after the pack command. Update only looks at nupkg not the exe/zip, but Scoop, Choco and WinGet will. Additionally, those two names are more user-friendly.

New-Item $output -ItemType Directory -Force
}


function Pack-Squirrel-Installer ($path, $version, $output) {
function Pack-Velopack-Installer($path, $version, $output)
{
# msbuild based installer generation is not working in appveyor, not sure why
Write-Host "Begin pack squirrel installer"
Copy link
Member

Choose a reason for hiding this comment

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

Please clean up any comments or references related to Squirrel installer

@jjw24
Copy link
Member

jjw24 commented Jun 21, 2024

Checking for new update throws an error message, maybe code hasn't handled when request returns no new version updates.

@taooceros
Copy link
Member Author

taooceros commented Jun 22, 2024

Checking for new update throws an error message, maybe code hasn't handled when request returns no new version updates.

I think that's the current behavior? We currently parse the github release ourselves. I don't think the current updater support pre-release...?

Copy link
Member

@jjw24 jjw24 left a comment

Choose a reason for hiding this comment

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

  1. Since we can not remove the target platform suffix for those files can you please remove them via code- '-win-x64-<xxxx>'.
    image

  2. Please also make the changes per the comments above.

@jjw24
Copy link
Member

jjw24 commented Jun 30, 2024

Checking for new update throws an error message, maybe code hasn't handled when request returns no new version updates.

I think that's the current behavior? We currently parse the github release ourselves. I don't think the current updater support pre-release...?

No, current behaviour is not throwing an error when there are no new updates. Please test check for update when there are no more updates.

@jjw24 jjw24 linked an issue Jul 4, 2024 that may be closed by this pull request
Copy link

github-actions bot commented Jul 4, 2024

@check-spelling-bot Report

🔴 Please review

See the 📂 files view, the 📜action log, or 📝 job summary for details.

❌ Errors Count
ℹ️ candidate-pattern 1
❌ ignored-expect-variant 1
⚠️ non-alpha-in-dictionary 13

See ❌ Event descriptions for more information.

Pattern suggestions ✂️ (1)

You could add these patterns to .github/actions/spelling/patterns.txt:

# Automatically suggested patterns

# hit-count: 1 file-count: 1
# Non-English
[a-zA-Z]*[ÀÁÂÃÄÅÆÇÈÉÊËÌÍÎÏÐÑÒÓÔÕÖØÙÚÛÜÝßàáâãäåæçèéêëìíîïðñòóôõöøùúûüýÿĀāŁłŃńŅņŒœŚśŠšŜŝŸŽžź][a-zA-Z]{3}[a-zA-ZÀÁÂÃÄÅÆÇÈÉÊËÌÍÎÏÐÑÒÓÔÕÖØÙÚÛÜÝßàáâãäåæçèéêëìíîïðñòóôõöøùúûüýÿĀāŁłŃńŅņŒœŚśŠšŜŝŸŽžź]*

If the flagged items are 🤯 false positives

If items relate to a ...

  • binary file (or some other file you wouldn't want to check at all).

    Please add a file path to the excludes.txt file matching the containing file.

    File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

    ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

  • well-formed pattern.

    If you can write a pattern that would match it,
    try adding it to the patterns.txt file.

    Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

    Note that patterns can't match multiline strings.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Outside diff range and nitpick comments (4)
README.md (4)

65-65: Clarify installation security warning.

The warning about security issues during the first-time installation could be clearer. Specify what the user should verify to ensure the downloaded file is safe.

> When installing for the first time, Windows may raise an issue about security due to the code not being signed. If you downloaded from this repo, then you are good to continue the setup. Or download the [early access version](https://github.com/Flow-Launcher/Prereleases/releases).
Tools
LanguageTool

[uncategorized] ~65-~65: When ‘set-up’ is used as a noun or modifier, it needs to be hyphenated.
Context: ...repo, then you are good to continue the set up. Or download the [early access version...

(VERB_NOUN_CONFUSION)


125-125: Clarify drag-and-drop behavior modification keys.

The description of how to modify the behavior of drag-and-drop operations using keyboard shortcuts could be more explicit about what each key does.

- Copy/move behavior can be changed via <kbd>Ctrl</kbd> or <kbd>Shift</kbd>, and the operation is displayed on the mouse cursor.
+ Copy/move behavior can be changed via <kbd>Ctrl</kbd> (copy) or <kbd>Shift</kbd> (move), and the operation is displayed on the mouse cursor.

207-207: Update user data folder path description for clarity.

The description of the user data folder path for the portable version should explicitly mention that it changes based on the installed version of FlowLauncher.

- If using portable, by default: `%LocalAppData%\FlowLauncher\app-<VersionOfYourFlowLauncher>\UserData`
+ If using the portable version, by default: `%LocalAppData%\FlowLauncher\app-<InstalledVersion>\UserData`

380-380: Add "please" for politeness.

The expression should include "please" to make it more polite.

- If you are unsure of a change you want to make, let us know in the [Discussions](https://github.com/Flow-Launcher/Flow.Launcher/discussions/categories/ideas), otherwise, please consider submitting a pull request.
+ If you are unsure of a change you want to make, please let us know in the [Discussions](https://github.com/Flow-Launcher/Flow.Launcher/discussions/categories/ideas), otherwise, please consider submitting a pull request.
Tools
LanguageTool

[style] ~380-~380: This expression usually appears with a “please” in front of it.
Context: ...re unsure of a change you want to make, let us know in the [Discussions](https://github.com...

(INSERT_PLEASE)

@jjw24 jjw24 removed the review in progress Indicates that a review is in progress for this PR label Aug 14, 2024
taooceros added a commit that referenced this pull request Sep 21, 2024
taooceros added a commit that referenced this pull request Sep 22, 2024
@taooceros taooceros mentioned this pull request Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Code Refactor enhancement New feature or request no-pr-activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove Parent Folder in Start Menu
4 participants