Skip to content
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

[lockfile-explorer] Add version validation capability (reopened) #4779

Merged
merged 53 commits into from
Jun 24, 2024

Conversation

L-Qun
Copy link
Contributor

@L-Qun L-Qun commented Jun 12, 2024

PR #4712 was accidentally merged before approval / passing CI.

Let's use this PR to conclude the review.

apps/lockfile-explorer/package.json Outdated Show resolved Hide resolved
apps/lockfile-explorer/src/commands/init.ts Outdated Show resolved Hide resolved
libraries/rush-lib/src/api/RushConfiguration.ts Outdated Show resolved Hide resolved
apps/lockfile-explorer/src/commands/init.ts Outdated Show resolved Hide resolved
apps/lockfile-explorer/src/commands/init.ts Outdated Show resolved Hide resolved
apps/lockfile-explorer/src/commands/lint.ts Outdated Show resolved Hide resolved
Comment on lines 93 to 107
await searchAndValidateDependencies(
rushConfiguration,
checkedProjects,
dependencyProject,
requiredVersions
);
}
} else {
await checkVersionCompatibility(
shrinkwrapFileMajorVersion,
packages,
fullDependencyPath,
requiredVersions,
checkedDependencyPaths
);
Copy link
Member

Choose a reason for hiding this comment

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

Seems like these results should be cached, as multiple projects can point at the same dependencies.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The same dependency is never checked twice

apps/lockfile-explorer/src/commands/lint.ts Outdated Show resolved Hide resolved
apps/lockfile-explorer/src/commands/lint.ts Outdated Show resolved Hide resolved
apps/lockfile-explorer/src/commands/init.ts Outdated Show resolved Hide resolved
Co-authored-by: Ian Clanton-Thuon <iclanton@users.noreply.github.com>
@L-Qun L-Qun requested a review from iclanton June 13, 2024 12:10
enable some parallelization
description: 'Show the full call stack if an error occurs while executing the tool'
});

this._subspaceParameter = this.defineStringParameter({
Copy link
Member

Choose a reason for hiding this comment

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

Putting this on the parser means that someone has to run lfx --subspace subspaceName start which feels awkward. lfs start --subspace subspaceName seems more ergonomic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This part has already been moved to Action.

@L-Qun L-Qun requested a review from iclanton June 18, 2024 09:09
L-Qun and others added 3 commits June 19, 2024 11:27
Co-authored-by: Ian Clanton-Thuon <iclanton@users.noreply.github.com>
Co-authored-by: Ian Clanton-Thuon <iclanton@users.noreply.github.com>
@L-Qun L-Qun requested a review from iclanton June 19, 2024 03:32
@octogonz octogonz merged commit 308c765 into microsoft:main Jun 24, 2024
5 checks passed
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.

None yet

3 participants