Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

Implemented tests for imageboard #40

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Implemented tests for imageboard #40

wants to merge 4 commits into from

Conversation

SamuelMarks
Copy link

Note: this isn't [yet] ready for being merged. PR created to facilitate discussion.

Questions:

  1. I've included typings, because that's what you've been doing. However, I've also added a tsd.json, so now people can get the package manager to populate that directory (like they would node_modules). Should I add typings to .gitignore?
  2. npm test now does a mocha --compilers ts:typescript-require -u exports -R spec tests/test_*.ts. Is that the approach you prefer?

Improving test coverage once these initial tests are in the folder should be trivial.

@DanielRosenwasser
Copy link
Member

Hey @SamuelMarks thanks for taking this on. I think the correct thing to do was just to use tsd.json. We're eventually going to remove the typings files, so if you could remove them altogether that would be ideal.

For the second question, I don't think we had a specific preference. If you have a better idea in mind, I think we'd welcome it. What were you thinking?

@SamuelMarks
Copy link
Author

@DanielRosenwasser: No worries. Okay, it's now ready to merge.

Would you like me to squash all my commits into one?

@DanielRosenwasser
Copy link
Member

@SamuelMarks no I think this is fine, though, I think other projects relied on the same node.d.ts. @mhegazy any idea about this?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants