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(cli.rs): pnpm tauri info exits with error (fix #2509) #2510

Merged
merged 8 commits into from
Aug 24, 2021

Conversation

linuxuser586
Copy link
Contributor

What kind of change does this PR introduce? (check at least one)

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

Does this PR introduce a breaking change? (check one)

  • Yes. Issue #___
  • No

The PR fulfills these requirements:

  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix: #xxx[,#xxx], where "xxx" is the issue number)
  • A change file is added if any packages will require a version bump due to this PR per the instructions in the readme.

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:

@linuxuser586 linuxuser586 requested a review from a team as a code owner August 21, 2021 23:40
@linuxuser586 linuxuser586 requested a review from a team August 21, 2021 23:40
Copy link
Member

@amrbashir amrbashir left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution, can you remove vfs dependency and use normal std::fs instead? seems to me it is not necessary to add vfs

tooling/cli.rs/src/info.rs Outdated Show resolved Hide resolved
tooling/cli.rs/src/info.rs Outdated Show resolved Hide resolved
@linuxuser586
Copy link
Contributor Author

@amrbashir I made the recommended changes. Please review.

Copy link
Member

@amrbashir amrbashir left a comment

Choose a reason for hiding this comment

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

Thanks again, The PR looks good to me but it might take a sometime to merge.

tooling/cli.rs/src/info.rs Outdated Show resolved Hide resolved
tooling/cli.rs/src/info.rs Outdated Show resolved Hide resolved
@linuxuser586
Copy link
Contributor Author

@amrbashir I updated the code to default to npm when no lock files exists since that was the behavior before my change.

@amrbashir
Copy link
Member

Nice, you basically read my mind.

@lucasfernog lucasfernog changed the base branch from dev to next August 24, 2021 15:27
@lucasfernog lucasfernog merged commit 2026134 into tauri-apps:next Aug 24, 2021
@chrisspiegl
Copy link

Is there a way to get the next branch working in a new project I am working on?

Or maybe release the current #next branch to the next version on NPM?

I am wondering cause it seems there are a lot of improvements since the latest beta release in august.

@FabianLars
Copy link
Member

FabianLars commented Dec 22, 2021

@chrisspiegl For the next cli you need to use the cargo based installation (cargo install tauri-cli --git https://github.com/tauri -> cargo tauri dev).

According to our release plans/guidelines, there won't be any next-only releases to npm and that wouldn't be enough anyway because the prebuilt clis (needed by the npm package) are pulled from a seperate github page.

@chrisspiegl
Copy link

chrisspiegl commented Dec 22, 2021

Thank you for clearing that up. I will try the cargo based cli.

For anyone wondering, I had to fix the cargo install a little bit and it worked to install with:

cargo install tauri-cli --git https://github.com/tauri-apps/tauri --branch next

now I can run cargo tauri info as well as cargo tauri dev and it works as expected with the current state.

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.

5 participants