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

feat(pnpm): allow latest as version to get the latest version #1933

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat(pnpm): allow latest as version to get the latest version #1933

wants to merge 2 commits into from
+229 −38

Conversation

gzm0
Copy link
Contributor

@gzm0 gzm0 commented Oct 4, 2024

Fixes #1907.

Changes are visible to end-users: yes

  • Searched for relevant documentation and updated as needed: yes
  • Breaking change (forces users to change their own code or config): no
  • Suggested release notes appear below: no

Test plan

  • Covered by existing test cases
  • New test cases added

@gzm0
test: unittest pnpm extension version resolution
226ddba 
This is in preparation for aspect-build#1907: As dependency resolution gets more
complex, we should test it.

Note: No updates to to bzl_library targets, because it seems
`extensions.bzl` isn't in a bzl_library.
@gzm0
feat(pnpm): allow latest as version to get the latest version
caa3916 
Fixes aspect-build#1907.
@gzm0 gzm0 marked this pull request as draft October 4, 2024 09:09
@aspect-workflows Aspect Workflows
Copy link

aspect-workflows bot commented Oct 4, 2024 •
edited
Loading

Test

1 test target passed

Targets 
//npm/private/test:test_pnpm_test_4 [k8-fastbuild]                46ms

Total test execution time was 46ms. 201 tests (99.5%) were fully cached saving 38s.

Test

e2e/bzlmod

All tests were cache hits

5 tests (100.0%) were fully cached saving 1s.

Test

e2e/gyp_no_install_script

All tests were cache hits

2 tests (100.0%) were fully cached saving 236ms.

Test

e2e/js_image_oci

All tests were cache hits

1 test (100.0%) was fully cached saving 7s.

Test

e2e/npm_link_package

All tests were cache hits

3 tests (100.0%) were fully cached saving 839ms.

Test

e2e/npm_link_package-esm

All tests were cache hits

3 tests (100.0%) were fully cached saving 745ms.

Test

e2e/npm_translate_lock

All tests were cache hits

1 test (100.0%) was fully cached saving 27ms.

Test

e2e/npm_translate_lock_empty

All tests were cache hits

1 test (100.0%) was fully cached saving 27ms.

Test

e2e/npm_translate_lock_multi

All tests were cache hits

2 tests (100.0%) were fully cached saving 296ms.

Test

e2e/npm_translate_lock_partial_clone

All tests were cache hits

1 test (100.0%) was fully cached saving 112ms.

Test

e2e/npm_translate_lock_replace_packages

All tests were cache hits

3 tests (100.0%) were fully cached saving 716ms.

Test

e2e/npm_translate_lock_subdir_patch

All tests were cache hits

1 test (100.0%) was fully cached saving 192ms.

Test

e2e/npm_translate_package_lock

All tests were cache hits

1 test (100.0%) was fully cached saving 81ms.

Test

e2e/npm_translate_yarn_lock

All tests were cache hits

1 test (100.0%) was fully cached saving 81ms.

Test

e2e/package_json_module

All tests were cache hits

1 test (100.0%) was fully cached saving 713ms.

Test

e2e/pnpm_lockfiles

All tests were cache hits

40 tests (100.0%) were fully cached saving 3s.

Test

e2e/pnpm_workspace

All tests were cache hits

10 tests (100.0%) were fully cached saving 2s.

Test

e2e/pnpm_workspace_rerooted

All tests were cache hits

10 tests (100.0%) were fully cached saving 2s.

Test

e2e/repo_mapping

All tests were cache hits

2 tests (100.0%) were fully cached saving 233ms.

Test

e2e/rules_foo

All tests were cache hits

2 tests (100.0%) were fully cached saving 153ms.

Test

e2e/vendored_node

All tests were cache hits

1 test (100.0%) was fully cached saving 171ms.

Buildifier      Format

@jbedard jbedard self-requested a review October 4, 2024 17:44
gregmagolan
gregmagolan approved these changes Oct 4, 2024
View reviewed changes
Copy link
Member

@gregmagolan gregmagolan left a comment

Choose a reason for hiding this comment

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

🌮

@gregmagolan gregmagolan marked this pull request as ready for review October 4, 2024 22:38
@gregmagolan
Copy link
Member

Looks good to me. Thanks @gzm0 !

@jbedard want to give it pass?

@jbedard
Copy link
Member

jbedard commented Oct 4, 2024

My only real question was in the prefactor: #1932 (comment)

But otherwise both PR LGTM 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gregmagolan gregmagolan gregmagolan approved these changes

@jbedard jbedard Awaiting requested review from jbedard

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

Successfully merging this pull request may close these issues.

[FR]: support latest keyword when loading pnpm in MODULE.bazel
3 participants
@gzm0 @gregmagolan @jbedard