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: automatic package manager install when autofixing #71

Merged
merged 23 commits into from
Aug 25, 2024

Conversation

Willem-Jaap
Copy link
Contributor

@Willem-Jaap Willem-Jaap commented Jun 29, 2024

Closes #38 & part of #68

This is my first time writing Rust so feel free to help me learn😅

Right now the implementation runs after --fix unless --no-install is passed and tries to detect the package manager.

image

image

Features:

  • Runs automatically after --fix or -f
  • Can be disabled using --no-install
  • Tries to detect the current package manager based on lockfile
  • Asks the user to choose a package manager if it can't be automatically determined
  • Runs the package manager command as a child process and streams the output

Copy link
Owner

@QuiiBz QuiiBz 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 the PR, and glad that it's your first time writing Rust!

Could you change the behavior to automatically run an install after --fix is run, unless we pass --no-install? We don't need to ask the user given we'll already have an option to disable it.

@Willem-Jaap
Copy link
Contributor Author

Install now runs automatically. Will add some more tests and polish when I have the time

@Willem-Jaap Willem-Jaap marked this pull request as ready for review July 7, 2024 10:39
@Willem-Jaap Willem-Jaap requested a review from QuiiBz July 7, 2024 10:39
@Willem-Jaap Willem-Jaap changed the title Auto install feat: auto install Aug 1, 2024
Copy link
Owner

@QuiiBz QuiiBz left a comment

Choose a reason for hiding this comment

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

Sorry for the huge delay on this PR, here's some feedback.

I wonder why we didn't already have -f as a shorthand for --fix, great addition 👍

src/install.rs Outdated Show resolved Hide resolved
src/install.rs Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@QuiiBz QuiiBz changed the title feat: auto install feat: automatic package manager install when autofixing Aug 24, 2024
Copy link
Owner

@QuiiBz QuiiBz left a comment

Choose a reason for hiding this comment

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

I added a few commits to add support for Bun & refactor a bit the code. Tested and seems to work fine, so I'll merge this soon. Thanks again for the help!

@Willem-Jaap
Copy link
Contributor Author

Great! Looking forward to having this merged

@QuiiBz QuiiBz merged commit 897cc53 into QuiiBz:main Aug 25, 2024
4 checks passed
@Willem-Jaap Willem-Jaap deleted the auto-install branch August 25, 2024 17:43
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.

feat?: --fix should run install command if fixing version missmatches
2 participants