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

fix: add repository metadata to all packages #3613

Merged
merged 3 commits into from
Oct 21, 2020
Merged

fix: add repository metadata to all packages #3613

merged 3 commits into from
Oct 21, 2020

Conversation

rarkins
Copy link
Contributor

@rarkins rarkins commented Oct 20, 2020

Closes #3612

Motivation

Ensure all npm packages now and in the future contain accurate repository metadata.

Have you read the [Contributing Guidelines on pull requests]

Yes

Test Plan

This PR includes its own test, so I think just code inspection is OK.

Related PRs

None

@facebook-github-bot
Copy link
Contributor

Hi @rarkins!

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Oct 20, 2020
@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

const glob = util.promisify(globCb);
const readFile = util.promisify(fsCb.readFile);

describe('packages', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Feel free to move/refactor/etc this test. Without it, things are likely to get out of sync again.

@docusaurus-bot
Copy link
Contributor

Deploy preview for docusaurus-2 ready!

Built without sensitive environment variables with commit 0cee011

https://deploy-preview-3613--docusaurus-2.netlify.app

@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Oct 20, 2020

Deploy preview for docusaurus-2 ready!

Built with commit 9e4b0ea

https://deploy-preview-3613--docusaurus-2.netlify.app

Copy link
Contributor

@lex111 lex111 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@lex111
Copy link
Contributor

lex111 commented Oct 20, 2020

9:20 error 'glob' should be listed in the project's dependencies. Run 'npm i -S glob' to add it import/no-extraneous-dependencies

There is one error to fix.

@rarkins
Copy link
Contributor Author

rarkins commented Oct 20, 2020

@lex111 it's in devDependencies, which is where I think it belongs. Maybe this lint error is because it's in packages/* and could be relocated?

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@rarkins
Copy link
Contributor Author

rarkins commented Oct 20, 2020

Sorry, I'm not sure how to fix this. Jest spec files shouldn't have a package.json, and glob shouldn't be added to the root package.json either. I tried moving the file into jest/ and also adding a eslint ignore rule, but nothing I tried stops the error message.

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

1 similar comment
@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@slorber slorber mentioned this pull request Oct 21, 2020
@slorber
Copy link
Collaborator

slorber commented Oct 21, 2020

Thanks, seems to be passing now ;)

@slorber slorber added the pr: maintenance This PR does not produce any behavior differences to end users when upgrading. label Oct 21, 2020
@slorber slorber merged commit 6930e91 into facebook:master Oct 21, 2020
@@ -100,6 +100,7 @@
"eslint-plugin-jsx-a11y": "^6.2.3",
"eslint-plugin-react": "^7.20.0",
"eslint-plugin-react-hooks": "^4.0.4",
"glob": "^7.1.6",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I was sure I'd add this already. Spent hours thinking it wanted me to move the dependency into dependencies and trying to fix that.. ugh

Copy link
Collaborator

Choose a reason for hiding this comment

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

:) maybe you didn't use the -W option to add dev dependencies in monorepo root ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: maintenance This PR does not produce any behavior differences to end users when upgrading.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

package.json repository/directory metadata incomplete
5 participants