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

Refactor to remove some uses of _.get and _.set #5327

Merged
merged 2 commits into from May 28, 2021

Conversation

stephenwade
Copy link
Contributor

Which issue, if any, is this issue related to?

Ref #4412.

Is there anything in the PR that needs further explanation?

Most of the code is self-explanatory. I replaced the simpler uses of _.get and _.set with native code.

I had to change a couple of array.forEach loops to regular for … of array loops to make tsc happy. I also updated the JSDoc types in a couple places for the same reason.

Copy link
Member

@ybiquitous ybiquitous left a comment

Choose a reason for hiding this comment

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

@stephenwade Thank you for making this change! 👏🏼
Could you check my review comments, please?

lib/assignDisabledRanges.js Outdated Show resolved Hide resolved
lib/normalizeAllRuleSettings.js Outdated Show resolved Hide resolved
lib/reportDisables.js Outdated Show resolved Hide resolved
lib/rules/max-line-length/index.js Outdated Show resolved Hide resolved
lib/utils/report.js Outdated Show resolved Hide resolved
lib/utils/report.js Outdated Show resolved Hide resolved
@jeddy3 jeddy3 changed the title Remove some uses of _.get and _.set Refactor to remove some uses of _.get and _.set May 28, 2021
@stephenwade
Copy link
Contributor Author

Hi @ybiquitous, I've implemented your suggested changes.

Copy link
Member

@ybiquitous ybiquitous left a comment

Choose a reason for hiding this comment

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

@stephenwade Thank you so much! LGTM 👍🏼

disabledRanges: {},
ruleSeverities: {},
customMessages: {},
};
Copy link
Member

Choose a reason for hiding this comment

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

[note] I'm a bit concerned about the newly added properties like disabledRanges, but it seems due to our TypeScript definitions (maybe another issue). 🤔

Anyway, I think no problems going on as-is. 👍🏼

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that was because of the TypeScript definitions. It wouldn't let me just set result.stylelint to {}.

Copy link
Member

@jeddy3 jeddy3 left a comment

Choose a reason for hiding this comment

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

Thanks!

@jeddy3 jeddy3 merged commit 0f6da86 into stylelint:v14 May 28, 2021
@stephenwade stephenwade deleted the remove-lodash-get-set-1 branch May 28, 2021 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants