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

Improve test setup #776

Merged
merged 3 commits into from
Mar 8, 2019
Merged

Improve test setup #776

merged 3 commits into from
Mar 8, 2019

Conversation

timaschew
Copy link
Member

Improving test setup which allows now to init and run docsify against fixture directories which include README.md and any other files. By default an empty README.md is used (for unit tests).

@docsifyjs/core feedback welcome

  • Make sure you are merging your commits to master branch.
  • Add some descriptions and refer relative issues for you PR.
  • DO NOT include files inside lib directory.

provide full test setup including fixtures
provide some dummy tests cases
@timaschew timaschew requested a review from a team February 25, 2019 06:20
@timaschew timaschew mentioned this pull request Mar 3, 2019
Copy link
Contributor

@jthegedus jthegedus left a comment

Choose a reason for hiding this comment

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

This looks good to me. (as stated elsewhere I believe we should get two reviews per PR to increase stability and spread knowledge)

My only concerns are the //TODOs that were removed, can you confirm these have been addressed by your implementation, or should they be documented to be addressed later in a different place?

@timaschew
Copy link
Member Author

@jthegedus
I converted one TODO into a NOTE. There are still two TODOs left in the tests and I would address them later. All others TODOs has been resolved.

@timaschew timaschew merged commit c1d4961 into master Mar 8, 2019
@anikethsaha anikethsaha deleted the improve-test-setup branch April 27, 2020 08:27
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.

3 participants