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

Added support for HTML (.html,.htm) #32

Merged
merged 5 commits into from
Aug 17, 2015
Merged

Added support for HTML (.html,.htm) #32

merged 5 commits into from
Aug 17, 2015

Conversation

dkebler
Copy link
Contributor

@dkebler dkebler commented Aug 13, 2015

Added support for HTML (.html,.htm) uses existing twig parser, updated read.me to reflect enhancement.

@dkebler
Copy link
Contributor Author

dkebler commented Aug 13, 2015

Whoops I didn't add tests. Doing that now (hopefully correctly, pretty nooby on writing test assertions).

@dkebler
Copy link
Contributor Author

dkebler commented Aug 13, 2015

Sorry I have no clue what the third parameter of the verifyComment function is supposed to be about so I don't know what to set it to (assuming it's what the error is about)

Can you fix this or tell me how. (see travisCL error)

  AssertionError: expected 1 to be 4

  + expected - actual

  -1

  +4

@pgilad
Copy link
Owner

pgilad commented Aug 13, 2015

third parameter is the line number of the comment, thanks for the PR btw

@dkebler
Copy link
Contributor Author

dkebler commented Aug 13, 2015

Well, with that tip and getting mocha running locally. It's ready now.
So sorry about all the commits. You can squash at your end?

Been self-learning node.js via a workflow project and also doing PRs is a good way to learn I think.
With this PR for your repo I learned a bit about mocha and can use your repo as a nice example to start including tests in mine so time well spent. So no, thank you.

@pgilad
Copy link
Owner

pgilad commented Aug 14, 2015

Thanks, will hopefully get to merge it soon when I'll have more time

@pgilad
Copy link
Owner

pgilad commented Aug 17, 2015

@dkebler Hey, still haven't gotten to this, will have time soon!

@pgilad pgilad merged commit c47dd97 into pgilad:master Aug 17, 2015
@pgilad
Copy link
Owner

pgilad commented Aug 17, 2015

Thanks!

@pgilad pgilad mentioned this pull request Aug 17, 2015
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