Skip to content

refactor: remove support for reloading lockfile on update#1701

Merged
jbedard merged 1 commit into
aspect-build:2.xfrom
jbedard:2.x-pnpm-no-reload
May 7, 2024
Merged

refactor: remove support for reloading lockfile on update#1701
jbedard merged 1 commit into
aspect-build:2.xfrom
jbedard:2.x-pnpm-no-reload

Conversation

@jbedard

@jbedard jbedard commented May 6, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

Reloading isn't supported with bzlmod so we should not support it with non-bzlmod just to make things consistent.

Close #1445

Changes are visible to end-users: yes/no

  • Breaking change (forces users to change their own code or config): yes
  • Suggested release notes appear below: yes

When npm_translate_lock(update_pnpm_lock) updates the lockfile it will exit with a non-0 exit code and must be rerun with the updated lockfile.

Test plan

  • Covered by existing test cases

@aspect-workflows

aspect-workflows Bot commented May 6, 2024 •
edited
Loading

Copy link
Copy Markdown

Test

All tests were cache hits

190 tests (100.0%) were fully cached saving 59s.

Test

e2e/bzlmod

All tests were cache hits

4 tests (100.0%) were fully cached saving 428ms.

Test

e2e/gyp_no_install_script

All tests were cache hits

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

Test

e2e/js_image_oci

All tests were cache hits

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

Test

e2e/npm_link_package

All tests were cache hits

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

Test

e2e/npm_link_package-esm

All tests were cache hits

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

Test

e2e/npm_translate_lock

All tests were cache hits

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

Test

e2e/npm_translate_lock_empty

All tests were cache hits

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

Test

e2e/npm_translate_lock_multi

All tests were cache hits

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

Test

e2e/npm_translate_lock_partial_clone

All tests were cache hits

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

Test

e2e/npm_translate_lock_subdir_patch

All tests were cache hits

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

Test

e2e/npm_translate_package_lock

All tests were cache hits

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

Test

e2e/npm_translate_yarn_lock

All tests were cache hits

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

Test

e2e/package_json_module

All tests were cache hits

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

Test

e2e/pnpm_lockfiles

All tests were cache hits

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

Test

e2e/pnpm_workspace

All tests were cache hits

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

Test

e2e/pnpm_workspace_rerooted

All tests were cache hits

6 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 467ms.

Test

e2e/rules_foo

All tests were cache hits

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

Test

e2e/vendored_node

All tests were cache hits

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

Buildifier      Format

@jbedard jbedard force-pushed the 2.x-pnpm-no-reload branch 2 times, most recently from 6ae502a to 0624a77 Compare May 6, 2024 06:30
@jbedard jbedard marked this pull request as ready for review May 6, 2024 06:35
@jbedard jbedard requested a review from gregmagolan May 6, 2024 06:35
@jbedard jbedard changed the base branch from main to 2.x May 6, 2024 06:37
@jbedard jbedard force-pushed the 2.x-pnpm-no-reload branch from 0624a77 to 89fbd80 Compare May 6, 2024 06:39
@gregmagolan gregmagolan mentioned this pull request May 6, 2024
Closed
21 tasks
@jbedard jbedard force-pushed the 2.x-pnpm-no-reload branch from 89fbd80 to d1fe919 Compare May 6, 2024 17:27
@jbedard jbedard force-pushed the 2.x-pnpm-no-reload branch 2 times, most recently from 00a9a44 to 67b6ab2 Compare May 6, 2024 19:23
gregmagolan
gregmagolan reviewed May 7, 2024
View reviewed changes
Comment thread e2e/update_pnpm_lock/test.sh Outdated
@jbedard jbedard force-pushed the 2.x-pnpm-no-reload branch from 67b6ab2 to 60e1311 Compare May 7, 2024 00:09
gregmagolan
gregmagolan reviewed May 7, 2024
View reviewed changes
Comment thread e2e/update_pnpm_lock/test.sh
@jbedard jbedard force-pushed the 2.x-pnpm-no-reload branch 2 times, most recently from def5af0 to 5fc719e Compare May 7, 2024 00:25
@jbedard jbedard requested a review from gregmagolan May 7, 2024 00:27
gregmagolan
gregmagolan reviewed May 7, 2024
View reviewed changes
Comment thread e2e/update_pnpm_lock/test.sh
gregmagolan
gregmagolan reviewed May 7, 2024
View reviewed changes
Comment thread e2e/update_pnpm_lock_with_import/test.sh
@jbedard jbedard force-pushed the 2.x-pnpm-no-reload branch from 5fc719e to 4589956 Compare May 7, 2024 00:30
@jbedard jbedard force-pushed the 2.x-pnpm-no-reload branch from 4589956 to b7919c1 Compare May 7, 2024 00:40
@jbedard jbedard requested a review from gregmagolan May 7, 2024 00:41
gregmagolan
gregmagolan approved these changes May 7, 2024
View reviewed changes

@gregmagolan gregmagolan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🌮

@jbedard jbedard merged commit ffd4583 into aspect-build:2.x May 7, 2024
@jbedard jbedard deleted the 2.x-pnpm-no-reload branch May 7, 2024 00:57
gregmagolan pushed a commit that referenced this pull request May 7, 2024
@jbedard
refactor: remove support for reloading lockfile on update (#1701)
da738bc
gregmagolan pushed a commit that referenced this pull request May 7, 2024
@jbedard
refactor: remove support for reloading lockfile on update (#1701)
2aaed9d
gregmagolan pushed a commit that referenced this pull request May 7, 2024
@jbedard
refactor: remove support for reloading lockfile on update (#1701)
1199e20
gregmagolan pushed a commit that referenced this pull request May 7, 2024
@jbedard
refactor: remove support for reloading lockfile on update (#1701)
1001223
gregmagolan pushed a commit that referenced this pull request May 7, 2024
@jbedard
refactor: remove support for reloading lockfile on update (#1701)
a405f27
jbedard added a commit that referenced this pull request May 8, 2024
@jbedard
refactor: remove support for reloading lockfile on update (#1701)
c7fdc6d
gregmagolan pushed a commit that referenced this pull request May 10, 2024
@jbedard
refactor: remove support for reloading lockfile on update (#1701)
abd9b92
gregmagolan pushed a commit that referenced this pull request May 10, 2024
@jbedard
refactor: remove support for reloading lockfile on update (#1701)
3a94e05
@gregmagolan gregmagolan mentioned this pull request May 10, 2024
Merged
@jbedard jbedard mentioned this pull request May 13, 2024
Closed
gregmagolan pushed a commit that referenced this pull request May 13, 2024
@jbedard
refactor: remove support for reloading lockfile on update (#1701)
4a87041
jbedard added a commit to jbedard/rules_js that referenced this pull request May 14, 2024
@jbedard
refactor: remove support for reloading lockfile on update (aspect-bui…
ded570c 
…ld#1701)
jbedard added a commit that referenced this pull request May 14, 2024
@jbedard
refactor: remove support for reloading lockfile on update (#1701)
64ed71d
jbedard added a commit to jbedard/rules_js that referenced this pull request May 16, 2024
@jbedard
refactor: remove support for reloading lockfile on update (aspect-bui…
e078e3b 
…ld#1701)
gregmagolan pushed a commit that referenced this pull request May 20, 2024
@jbedard
refactor: remove support for reloading lockfile on update (#1701)
4013307
gregmagolan pushed a commit that referenced this pull request May 20, 2024
@jbedard
refactor: remove support for reloading lockfile on update (#1701)
a9eeb36
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

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jbedard @gregmagolan