Skip to content

Conversation

gribnoysup
Copy link
Collaborator

@gribnoysup gribnoysup commented Aug 23, 2021

I noticed that our CONTRIBUTING section about creating a new package got kinda long and consisting of too many steps to do manually, there was also some undocumented things about the npm scripts that caused some CI failures recently. I started with just updating the docs, but it seems to me that having some tool that will ask you a few questions and bootstrap everything itself might be a better approach here. This is how this script looks in action:

Kapture 2021-08-23 at 11 55 59

I also moved all the config files into their own configs folder so it's a bit easier to find them and they are not in the way when looking for packages.

Rendered CONTRIBUTING.md for convenience.

@gribnoysup gribnoysup changed the title yaml arraysWorkspace template Workspace template Aug 23, 2021
@gribnoysup gribnoysup changed the title Workspace template feat(create-workspace): Add a script to quickly create a new workspace from the template Aug 23, 2021
Copy link
Collaborator

@mcasimir mcasimir left a comment

Choose a reason for hiding this comment

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

Amazing! Great to have these config packages in its own folder, and the mocha config is super neat.

"prepublishOnly": "npm run compile",
"pretest": "npm run compile && mongodb-runner start --port=27018",
"test": "cross-env TS_NODE_FILES=true nyc mocha --colors --timeout 15000 -r ts-node/register src/**/*.spec.*",
"test": "cross-env TS_NODE_FILES=true nyc mocha",
Copy link
Collaborator

Choose a reason for hiding this comment

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

was fighting with this a lot, can we have a different task for covered tests (test-cov)? And a test-watch with --watch, maybe only me, but personally I use that a ton and run tests each 3 seconds, i would love to have a way for that to be really fast.

depcheck: 'depcheck',
check: 'npm run lint && npm run depcheck',
'check-ci': 'npm run check',
test: 'cross-env TS_NODE_FILES=true nyc mocha',
Copy link
Collaborator

@mcasimir mcasimir Aug 23, 2021

Choose a reason for hiding this comment

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

was fighting with this a lot, can we have a different task for covered tests (test-cov)? And a test-watch with --watch, maybe is only me, but personally I use that a ton and run tests each 3 seconds, i would love to have a way for that to be really fast.

Would you be ok with including the changes i've done here: (also fixes nyc reporting coverage in test files)

https://github.com/mongodb-js/compass/pull/2412/files#diff-a557af5d61219f0b3f0b1fc6810efa3a9e0889fc54005cd248908a50cdf1d612L40

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Totally, let's include this stuff in the template! Then following your suggestion we will scripts like this:

  • test -> mocha
  • test-cov -> cross-env TS_NODE_FILES=true nyc -x "**/*.spec.*" npm run test
  • test-watch -> npm run test -- --watch
  • test-ci -> npm run test-cov

Makes sense?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done in 34089b9

return;
}

console.log();
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this a newline?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep

...base,
require: base.require.concat([
'jsdom-global/register',
path.resolve(__dirname, 'chai-dom-register.js'),
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we add also sinon and sinon-chai somewhere?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, good idea, seems like we use it a lot in tests suites (I somehow never noticed before)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done in a74c870

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.

2 participants