Skip to content

Inline fix logic#47

Merged
wraithgar merged 4 commits intomainfrom
gar/legacy-fixer
Jul 7, 2023
Merged

Inline fix logic#47
wraithgar merged 4 commits intomainfrom
gar/legacy-fixer

Conversation

@wraithgar
Copy link
Copy Markdown
Contributor

@wraithgar wraithgar requested a review from a team as a code owner July 7, 2023 15:14
Comment thread lib/normalize.js Outdated
Comment thread lib/normalize.js
@lukekarrys
Copy link
Copy Markdown
Contributor

Looks good! One minor swap for a var -> const and a suggestion.

wraithgar added 2 commits July 7, 2023 11:20
This brings in the logic from `normalize-package-data` to be in this repo
instead.  It doesn't bring all of the logic, just the steps involved
with "fix".

There was some re-duplication of `bundleDependencies` that
`normalize-package-data` was doing that has been removed.  It was also
completely re-implementing the script fixing, which we already call as
part of "fix" so that was dropped.
@wraithgar wraithgar merged commit 3bcf2fd into main Jul 7, 2023
@wraithgar wraithgar deleted the gar/legacy-fixer branch July 7, 2023 18:24
@github-actions github-actions Bot mentioned this pull request Jul 7, 2023
Comment thread lib/normalize.js
if (!data.version) {
data.version = ''
} else {
if (!semver.valid(data.version, !loose)) {
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.

(Here and line 80) bool arg is loose based on typings: https://unpkg.com/browse/@types/[email protected]/functions/valid.d.ts

Suggested change
if (!semver.valid(data.version, !loose)) {
if (!semver.valid(data.version, loose)) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yep this is a typo from refactoring, I'd changed it to !strict but then decided to keep the loose param.

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.

3 participants