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.

Sign up for GitHub

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

Jump to bottom

Fixes #7767: Updated bun upgrade to return error when used with cmd arguments #7784

Merged
merged 20 commits into from
Mar 4, 2024
Merged

Fixes #7767: Updated bun upgrade to return error when used with cmd arguments #7784

merged 20 commits into from
Mar 4, 2024

Conversation

BrookJeynes
Copy link
Contributor

@BrookJeynes BrookJeynes commented Dec 22, 2023 •
edited
Loading

What does this PR do?

Updated the upgrade command to check for command-line arguments and return an error if so providing a suggested update command.

fixes #7767

❯ bun upgrade bun-types --dev
error: this command updates bun itself, and does not take package names.
Use `bun update bun-types --dev` instead.
  • Code changes

How did you verify your code works?

I wrote automated tests

If Zig files changed:

  • I checked the lifetime of memory allocated to verify it's (1) freed and (2) only freed when it should be
  • I included a test for the new code, or an existing test covers it

@BrookJeynes
Updated upgrade to return error when used with cmd arguments
d92dddf 
- Updated `upgrade` command to check for command-line arguments and return an error if so providing a suggested `update` command.
@BrookJeynes
added test
ddbf571
@BrookJeynes BrookJeynes changed the title 7767 better upgrade dx 7767 - Updated upgrade to return error when used with cmd arguments Dec 22, 2023
@BrookJeynes BrookJeynes changed the title 7767 - Updated upgrade to return error when used with cmd arguments Fix: #7767 - Updated bun upgrade to return error when used with cmd arguments Dec 22, 2023
@BrookJeynes BrookJeynes changed the title Fix: #7767 - Updated bun upgrade to return error when used with cmd arguments Fixes #7767: Updated bun upgrade to return error when used with cmd arguments Dec 22, 2023
paperclover
paperclover requested changes Jan 8, 2024
View reviewed changes
Copy link
Member

@paperclover paperclover left a comment

Choose a reason for hiding this comment

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

I love this feature. I remember discussion before about confusion between "update" and "upgrade" but this feature is nice solution to the ambiguity.

there is a critical mistake in the test files, so i will delay running ci until that is fixed

test/cli/install/bun-upgrade.test.ts Outdated Show resolved Hide resolved
src/cli.zig Outdated Show resolved Hide resolved
src/cli.zig Outdated Show resolved Hide resolved
paperclover
paperclover approved these changes Feb 13, 2024
View reviewed changes
Copy link
Member

@paperclover paperclover left a comment

Choose a reason for hiding this comment

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

i apologize for long delay in review. this looks alright.

autofix-ci bot and others added 4 commits February 13, 2024 19:41
@autofix-ci
[autofix.ci] apply automated fixes
3d03d49
@BrookJeynes
Merge branch 'main' into 7767-better-upgrade-dx
aa94735
@BrookJeynes
Fixed build errors
221c874 
- Removed call to `std.mem.span`
- Added conditional to only run if there's more than 2 arguments (ignores the exec and `upgrade`)
- Added new test to ensure `upgrade` runs with 0 arguments passed
@paperclover
Merge branch 'main' into 7767-better-upgrade-dx
0ce015f
@paperclover paperclover merged commit fa08ac3 into oven-sh:main Mar 4, 2024
27 of 31 checks passed
@BrookJeynes BrookJeynes deleted the 7767-better-upgrade-dx branch March 4, 2024 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paperclover paperclover paperclover approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Provide better DX when someone confuses bun update with bun upgrade
2 participants
@BrookJeynes @paperclover