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(cli/mobile/init): skip installing already installed targets, closes #7044 #7058

Merged
merged 4 commits into from
May 26, 2023

Conversation

amrbashir
Copy link
Member

@amrbashir amrbashir commented May 25, 2023

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 May 25, 2023 14:51
amrbashir and others added 2 commits May 25, 2023 18:20
Co-authored-by: Lucas Fernandes Nogueira <lucas@tauri.studio>
@lucasfernog lucasfernog changed the title feat(cli/mobile/init): add --skip-targets-install, closes #7044 feat(cli/mobile/init): skip installing already installed targets, closes #7044 May 25, 2023
@lucasfernog
Copy link
Member

@amrbashir I think it's easier if we detect installed targets instead. Let me know if you have something against it.

@amrbashir
Copy link
Member Author

It is a good addition but I still think we should provide the flag anyways. Some users may want to install only install 1 or 2 targets and the flag would allow them to do so.

@lucasfernog
Copy link
Member

The flag sounds a little feature creep to me, I would avoid unless it's really necessary.

This check is already addressing a super particular use case :D usually you'll just want everything installed for you to just run android build and expect it to work flawlessly.

@amrbashir
Copy link
Member Author

Ideally, tauri dev, tauri build, tauri android dev, tauri android build...etc would check and install the need requirements if possible but would, at the same time, provide a flag to skip this part, otherwise unusual environments like Nix will fail.

@lucasfernog
Copy link
Member

Yeah but if there's a good way to detect instead of providing a skip flag.. 😉

@amrbashir
Copy link
Member Author

Detection always has the possibility of missing 0.01% of users and these are the users that open github issues that will ask for a flag. I like detecting and auto fixes but at the same time I also like being in control and as tauri-cli is considered a build tool, everything it does should be configurable IMO. And I don't really consider it a feature creep since the implementation is actually simple and doesn't affect anything else.

@lucasfernog
Copy link
Member

I tend do go with the wait and see approach in these cases.. like only adding that check when it's requested and there's no better way to do so. But we can open another PR to continue the discussion.

@lucasfernog lucasfernog merged commit a28fdf7 into next May 26, 2023
13 checks passed
@lucasfernog lucasfernog deleted the feat/cli/skip-target-install branch May 26, 2023 02:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🔎 In audit
Development

Successfully merging this pull request may close these issues.

[feat] skip rustup installation for cargo tauri android init (for nix user)
2 participants