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-core-utils): port physical cpu count to ts #22031

Closed
wants to merge 1 commit into from
Closed

chore(gatsby-core-utils): port physical cpu count to ts #22031

wants to merge 1 commit into from

Conversation

aminkhp
Copy link

@aminkhp aminkhp commented Mar 7, 2020

Description

port physical-cpu-count.js to typescript

Related Issues

Related to #21995

@aminkhp aminkhp requested a review from a team as a code owner March 7, 2020 18:54
@@ -52,4 +52,4 @@ function getPhysicalCpuCount() {
return fallbackToNodeJSCheck()
}

module.exports = getPhysicalCpuCount()
export default getPhysicalCpuCount()
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use Guidelines (from #21995 description):

Only use named exports, no export default.

Suggested change
export default getPhysicalCpuCount()
export const physicalCpuCount = getPhysicalCpuCount()

and adjust all physical-cpu-count imports?

That should be in

let coreCount = require(`./physical-cpu-count`) || 1

and various tests

Copy link
Contributor

@ascorbic ascorbic left a comment

Choose a reason for hiding this comment

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

Please make the change to named exports as per @pieh 's suggestion

@blainekasten
Copy link
Contributor

Sorry, I hijacked this change in #22122

Thank you so much for contributing to our TypeScript refactor! We have more work to do and we would love to have you stay involved in our transition. Please submit more PRs! 💜

@aminkhp aminkhp deleted the chore/gcu_ts1 branch May 10, 2020 13:01
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.

5 participants