Skip to content

Conversation

trivikr
Copy link
Contributor

@trivikr trivikr commented Mar 29, 2022

What's the problem this PR addresses?

The variable descriptor can be declared as const, and assigned values in a conditional operator. Another const can be used for initial parsed descriptor.

How did you fix it?

Use const for variable descriptor, and introduce new const parsedDescriptor

Checklist

  • I have set the packages that need to be released for my changes to be effective.
  • I will check that all automated PR checks pass before the PR gets reviewed.

@trivikr trivikr requested a review from arcanis as a code owner March 29, 2022 20:37
@trivikr trivikr force-pushed the trivikr/core/const-descriptor branch 3 times, most recently from 370247a to 7e1b364 Compare April 6, 2022 23:43
@trivikr trivikr force-pushed the trivikr/core/const-descriptor branch 2 times, most recently from b0c3ebb to 56daa8c Compare May 18, 2022 03:43
@trivikr trivikr force-pushed the trivikr/core/const-descriptor branch from 56daa8c to 0cf02fe Compare May 24, 2022 06:23
@merceyz merceyz force-pushed the trivikr/core/const-descriptor branch from 0cf02fe to 160f835 Compare July 12, 2022 12:11
merceyz
merceyz previously approved these changes Jul 12, 2022
@merceyz merceyz changed the title chore(core): use const for descriptor in LegacyMigrationResolver refactor(core): use const for descriptor in LegacyMigrationResolver Jul 12, 2022
@merceyz merceyz enabled auto-merge (squash) July 12, 2022 12:42
auto-merge was automatically disabled July 18, 2022 02:46

Head branch was pushed to by a user without write access

@trivikr trivikr force-pushed the trivikr/core/const-descriptor branch from bf1c242 to 6542f01 Compare July 18, 2022 02:46
@trivikr trivikr requested a review from merceyz July 18, 2022 02:47
merceyz
merceyz previously approved these changes Jul 18, 2022
Copy link
Member

@merceyz merceyz left a comment

Choose a reason for hiding this comment

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

Still LGTM, can't merge without @arcanis (the code owner) approving though.

@merceyz merceyz enabled auto-merge (squash) July 18, 2022 21:57
auto-merge was automatically disabled September 26, 2022 23:07

Head branch was pushed to by a user without write access

@trivikr trivikr force-pushed the trivikr/core/const-descriptor branch from 6542f01 to 38d367e Compare September 26, 2022 23:07
@merceyz merceyz merged commit ea5e63a into yarnpkg:master Oct 5, 2022
@trivikr trivikr deleted the trivikr/core/const-descriptor branch October 5, 2022 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants