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

feat(bundler/msi): add perMachine option, disabled by default, closes #2319 #4509

Closed
wants to merge 1 commit into from

Conversation

amrbashir
Copy link
Member

@amrbashir amrbashir commented Jun 28, 2022

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Docs
  • New Binding issue #___
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change?

  • Yes, and the changes were approved in issue #___
  • No

Checklist

  • When resolving issues, they are referenced in the PR's title (e.g fix: remove a typo, closes #___, #___)
  • A change file is added if any packages will require a version bump due to this PR per the instructions in the readme.
  • I have added a convincing reason for adding this feature, if necessary

Other information

@amrbashir amrbashir requested a review from a team as a code owner June 28, 2022 18:16
---

Added `perMachine` option to `tauri.conf.json > tauri > bundle > windows > wix`, disabled by default.
**Breaking change** `msi` installer will install your app per-user by default now.
Copy link
Member

Choose a reason for hiding this comment

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

not really a breaking change right?

Copy link
Member Author

@amrbashir amrbashir Jun 28, 2022

Choose a reason for hiding this comment

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

It is a slight breaking change, that's why I kept the bump as patch (still considering to change it to minor).

Apps that relied heavily on being installed perMachine won't work unless they specify perMachine. I don't have an example at the top of my head but I thought I'd be cautious and treat it as breaking.

Copy link
Member

Choose a reason for hiding this comment

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

doesn't it make more sense to keep it enabled by default then?

Edit: Actually i think i prefer perUser being the default, nvm me

Copy link
Member

Choose a reason for hiding this comment

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

I prefer perUser as default but if it can break any kind of app, we need to keep the per machine as default.

Copy link
Member Author

Choose a reason for hiding this comment

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

it has a slight chance to break some apps but I think perUser should be the default, even if it means this PR shouldn't be merged in v1

Copy link
Member

Choose a reason for hiding this comment

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

even tho i can't think of anything really where changing it to perUser would break anything, but what about merging this pr with perMachine as default and changing the default with v2?

Edit: Okay it can break if the user selects a folder like Program Files, because it doesn't request permissions on demand (is that even possible?)

Copy link

Choose a reason for hiding this comment

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

I have to move my glow stick for you guys.
I think this is a very important function and it will be very helpul if it can be merged and published as soon as possible.

@amrbashir
Copy link
Member Author

Only tested with examples/api, will test examples/resources and examples/sidecar in a bit.

@amrbashir amrbashir marked this pull request as draft June 28, 2022 20:27
@amrbashir amrbashir closed this Jun 29, 2022
@FabianLars FabianLars deleted the wix-per-user branch July 2, 2022 05:27
@FabianLars FabianLars mentioned this pull request Jul 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants