Skip to content

[rush-pnpm] Fix rush-pnpm patch-commit command #4874

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

Merged
merged 3 commits into from
Aug 10, 2024
Merged

[rush-pnpm] Fix rush-pnpm patch-commit command #4874

merged 3 commits into from
Aug 10, 2024

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
5 checks passed
@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