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

chore(gatsby-source-contentful): drop last usages of lodash #28441

Merged
merged 2 commits into from
Dec 2, 2020
Merged

Conversation

pvdz
Copy link
Contributor

@pvdz pvdz commented Dec 2, 2020

Drops the last remaining cases of lodash from this file.

See inline comments for clarification

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Dec 2, 2020
@@ -178,11 +176,8 @@ const resolveFixed = (image, options) => {
)
})

// Sort sizes for prettiness.
const sortedSizes = _.sortBy(filteredSizes)
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 array is initialized ordered. It is only pruned, not shuffled / added. So this sort should be a noop in any case.

Copy link
Contributor

Choose a reason for hiding this comment

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

This warrants a comment that fixedSizes is expected to be sorted in the first place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure

}

// Sort sizes for prettiness.
const sortedSizes = _.sortBy(filteredSizes)
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 sort is similar to above except the condition before it does not exist in the other case.

The condition will only add the current width if it doesn't already exist here. And the loop before will cull any widths that are greater than this width. So either way, the width should be the last element in the list. This means the .sortBy is equally useless here and is safe to drop.

@pvdz pvdz added topic: source-contentful Related to Gatsby's integration with Contentful type: chore and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Dec 2, 2020
@delete-merged-branch delete-merged-branch bot deleted the branch master December 2, 2020 14:16
@pvdz pvdz changed the base branch from notpicky to master December 2, 2020 14:18
@pvdz
Copy link
Contributor Author

pvdz commented Dec 2, 2020

Ok. This branch switching is a little funky :/

Copy link
Contributor

@vladar vladar left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

P.S. Love the branch name 😄

@pvdz pvdz merged commit 1853f3b into master Dec 2, 2020
@delete-merged-branch delete-merged-branch bot deleted the nodash branch December 2, 2020 15:02
pragmaticpat pushed a commit to pragmaticpat/gatsby that referenced this pull request Apr 28, 2022
…#28441)

* chore(gatsby-source-contentful): drop last usages of lodash

* Update packages/gatsby-source-contentful/src/extend-node-type.js

Co-authored-by: Vladimir Razuvaev <[email protected]>

Co-authored-by: Vladimir Razuvaev <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: source-contentful Related to Gatsby's integration with Contentful
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants