Skip to content

Conversation

@L-Qun
Copy link
Contributor

@L-Qun L-Qun commented Aug 9, 2024

Details

After upgrading the rush version(5.131.0) in tiktok monorepo, we encountered an error when running rush-pnpm --subspace subspaceName patch-commit xxxx

img_v3_02di_2ae44f28-61d6-4bcf-91e1-c2968dbfb3hu

After taking a close look at the rush source code, I found that the path to the folder related to the pnpm patches was incorrect. Therefore, I fixed this issue in that MR

How it was tested

To test the implementation, the following commands were executed:

  1. rush install -t .
  2. rush-pnpm --subspace subspaceName patch xxx
  3. rush-pnpm --subspace subspaceName patch-commit xxx

No errors were encountered.

Impacted documentation

None

@L-Qun
Copy link
Contributor Author

L-Qun commented Aug 9, 2024

@iclanton Pete is on vacation. We need your help with the review and merge

@L-Qun L-Qun changed the title Fix rush-pnpm patch-commit [rush-pnpm] Fix rush-pnpm patch-commit command Aug 9, 2024
Copy link
Member

@D4N14L D4N14L left a comment

Choose a reason for hiding this comment

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

@L-Qun have you validated that this still works in non-subspaces scenarios (or I guess, the default subspace)?

@L-Qun
Copy link
Contributor Author

L-Qun commented Aug 9, 2024

@L-Qun have you validated that this still works in non-subspaces scenarios (or I guess, the default subspace)?

@D4N14L Yeah, I tested it. Everything works well

image

image

@L-Qun L-Qun requested a review from D4N14L August 9, 2024 17:26
@iclanton iclanton enabled auto-merge (squash) August 10, 2024 01:04
auto-merge was automatically disabled August 10, 2024 01:46

Head branch was pushed to by a user without write access

….json

Co-authored-by: Ian Clanton-Thuon <iclanton@users.noreply.github.com>
@L-Qun
Copy link
Contributor Author

L-Qun commented Aug 10, 2024

@iclanton Could you help to publish a new rush version

@iclanton iclanton merged commit fceee8e into microsoft:main Aug 10, 2024
@iclanton
Copy link
Member

@L-Qun Published in Rush v5.131.3

@L-Qun
Copy link
Contributor Author

L-Qun commented Aug 10, 2024

@L-Qun Published in Rush v5.131.3

Thanks @iclanton

@iclanton
Copy link
Member

@L-Qun - Looks like this PR introduced a breaking change. In the non-subspaces scenario, the patches folder is now common/config/rush/pnpm-patches, and before it was common/config/pnpm-patches

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

Labels

None yet

Projects

Status: Closed

Development

Successfully merging this pull request may close these issues.

3 participants