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

Allow comments at end of Example tests #39

Merged
merged 1 commit into from
Oct 14, 2019
Merged

Allow comments at end of Example tests #39

merged 1 commit into from
Oct 14, 2019

Conversation

hairyhenderson
Copy link
Contributor

Fixes #38

I'm not sure if this is the cleanest way to fix it, but it was the most direct path I could hack together over a few minutes 😉

Signed-off-by: Dave Henderson dhenderson@gmail.com

Signed-off-by: Dave Henderson <dhenderson@gmail.com>
@coveralls
Copy link

Pull Request Test Coverage Report for Build 110

  • 11 of 11 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.3%) to 86.655%

Totals Coverage Status
Change from base Build 106: 0.3%
Covered Lines: 974
Relevant Lines: 1124

💛 - Coveralls

1 similar comment
@coveralls
Copy link

Pull Request Test Coverage Report for Build 110

  • 11 of 11 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.3%) to 86.655%

Totals Coverage Status
Change from base Build 106: 0.3%
Covered Lines: 974
Relevant Lines: 1124

💛 - Coveralls

@coveralls
Copy link

coveralls commented Oct 14, 2019

Pull Request Test Coverage Report for Build 108

  • 11 of 11 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.3%) to 86.655%

Totals Coverage Status
Change from base Build 106: 0.3%
Covered Lines: 974
Relevant Lines: 1124

💛 - Coveralls

@bombsimon
Copy link
Owner

Thanks, I think this is a reasonable path and I had a similar way in mind. I like that you tested both with and without a newline! 👍

This is a naive approach since this handles more cases than just output comments but I guess it's easy to break things if you want to. To be sure one might parse the comment map and ensure that the trailing comment includes (Unordered )?[Oo]utput but that might be overkill. Another thing that will pass now is a trailing whitespace after the output comments but I think that's ok for now too!

@bombsimon bombsimon merged commit 7e5bf66 into bombsimon:master Oct 14, 2019
@hairyhenderson hairyhenderson deleted the allow-comments-at-end-of-examples-38 branch October 14, 2019 16:03
@hairyhenderson
Copy link
Contributor Author

Thanks @bombsimon, that was fast! 🙂

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.

Correct Example syntax triggering "block should not end with a whitespace (or comment)"
3 participants