Skip to content

rush update --full#13384

Merged
1 commit merged intoAzure:masterfrom
xirzec:updateLockfile
Jan 25, 2021
Merged

rush update --full#13384
1 commit merged intoAzure:masterfrom
xirzec:updateLockfile

Conversation

@xirzec
Copy link
Copy Markdown
Member

@xirzec xirzec commented Jan 25, 2021

#13353 once again re-introduced karma-remap-instanbul into our lockfile, perhaps due to bad merge from master. This PR hopefully will resolve this for the last time.

@xirzec xirzec requested a review from bterlson as a code owner January 25, 2021 22:16
@xirzec xirzec added auto-merge Client This issue points to a problem in the data-plane of the library. labels Jan 25, 2021
@ghost
Copy link
Copy Markdown

ghost commented Jan 25, 2021

Hello @xirzec!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@@ -849,34 +849,34 @@ packages:
integrity: sha512-mky/O83TXmGY39P1H9YbUpjV6l6voRYlufqfFCvel8l1phuy8HRjdWc1rrPuN53ITBJlbyMSV6z3niOySO5pgQ==
/@types/fs-extra/8.1.1:
dependencies:
'@types/node': 10.17.51
'@types/node': 8.10.66
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this supposed to go down?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

My understanding was this was bumped up accidentally by this PR: 189bb99#diff-5029779360a5dab7df00ff76c5693ce33b68f76d2f433b44a28b333bf33f674a

/cc @ramya-rao-a @jonathandturner

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It was fixed in a previous commit I did too: 24cbc28#diff-5029779360a5dab7df00ff76c5693ce33b68f76d2f433b44a28b333bf33f674a

Looks like we've had a spate of folks not reverting the lockfile to master before running rush update when there's a conflict?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

I think the following two observations might have to do with this:

  • The dependencies for @types packages are typically other @types packages with the semvar being just "*". For example, if you look at common/temp/node_modules/@types/fs-extra/package.json, you will see the following:
 "dependencies": {
        "@types/node": "*"
    }

this introduces issues such as microsoft/rushstack#750.

@ramya-rao-a
Copy link
Copy Markdown
Contributor

cc @sarangan12

@xirzec, What could we have done to prevent the regression?

@xirzec
Copy link
Copy Markdown
Member Author

xirzec commented Jan 25, 2021

@xirzec, What could we have done to prevent the regression?

I gotta figure out if rush is just not doing the right thing, or if I can document a process for handling pnpm-lockfile conflicts that will work for everyone.

@ghost ghost merged commit d87b7b2 into Azure:master Jan 25, 2021
@xirzec xirzec deleted the updateLockfile branch January 25, 2021 23:30
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Client This issue points to a problem in the data-plane of the library.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants