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

3370: Add support for comment lines in "mocha.opts" #3375

Merged
merged 2 commits into from
May 8, 2018

Conversation

plroebuck
Copy link
Contributor

Description of the Change

Modified code to strip comment lines [lines beginning with a hash character ('#')] from "mocha.opts" prior to processing its contents. Inline comments are not supported.

Alternate Designs

See related issues in #3370.
One used JS comments '//', but most everyone agreed hash '#' was more fitting.
Others got bogged down attempting to figure out how to support inline comments.

Why should this be in core?

N/A

Benefits

  • Allows user to document the arguments in the options file.
  • An argument can now be commented out, but left in file. (user request)

Possible Drawbacks

None to knowledge

Applicable issues

Fixes #3370
(minor/patch release)? yes

This is an enhancement with no impact on production code; could go with either.

Modified code to strip comment lines, those beginning with a hash character ('#'), from "mocha.opts"
prior to processing its contents.

Fixes mochajs#3370
@plroebuck plroebuck closed this May 3, 2018
@plroebuck plroebuck changed the title feat(bin/options.js): Add support for comment lines in "mocha.opts" 3370: Add support for comment lines in "mocha.opts" May 3, 2018
@plroebuck plroebuck reopened this May 3, 2018
@coveralls
Copy link

coveralls commented May 3, 2018

Coverage Status

Coverage increased (+0.05%) to 90.054% when pulling ddd15ed on plroebuck:issue/3370 into 7613521 on mochajs:master.

@boneskull boneskull force-pushed the master branch 2 times, most recently from 4547268 to 7613521 Compare May 4, 2018 16:47
Copy link
Contributor

@outsideris outsideris left a comment

Choose a reason for hiding this comment

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

Describing comments on docs will be great.

@outsideris outsideris added nice to have enhancement proposal of low priority area: usability concerning user experience or interface labels May 5, 2018
Added information on file content, noting support for comment/blank lines.
@outsideris outsideris added the semver-minor implementation requires increase of "minor" version number; "features" label May 6, 2018
Copy link
Contributor

@Bamieh Bamieh left a comment

Choose a reason for hiding this comment

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

awesome, good job buddy

@plroebuck
Copy link
Contributor Author

@boneskull ping

@Bamieh Bamieh merged commit aef1b0f into mochajs:master May 8, 2018
@boneskull boneskull added this to the next milestone May 17, 2018
wopian referenced this pull request in wopian/agc-assembly May 20, 2018
This Pull Request updates dependency [mocha](https://github.com/mochajs/mocha) from `~5.1.0` to `~5.2.0`



<details>
<summary>Release Notes</summary>

### [`v5.2.0`](https://github.com/mochajs/mocha/blob/master/CHANGELOG.md#&#8203;520--2018-05-18)
[Compare Source](mochajs/mocha@v5.1.1...v5.2.0)
#### 🎉 Enhancements

- [#&#8203;3375]: Add support for comments in `mocha.opts` ([@&#8203;plroebuck])
#### 🐛 Fixes

- [#&#8203;3346]: Exit correctly from `before` hooks when using `--bail` ([@&#8203;outsideris])
#### 📖 Documentation

- [#&#8203;3328]: Mocha-flavored [API docs](https://mochajs.org/api/)! ([@&#8203;Munter])
#### 🔩 Other

- [#&#8203;3330]: Use `Buffer.from()` ([@&#8203;harrysarson])
- [#&#8203;3295]: Remove redundant folder ([@&#8203;DavNej])
- [#&#8203;3356](`https://github.com/mochajs/mocha/pull/3356`): Refactoring ([@&#8203;plroebuck])

[#&#8203;3375]: `https://github.com/mochajs/mocha/pull/3375`
[#&#8203;3346]: `https://github.com/mochajs/mocha/pull/3346`
[#&#8203;3328]: `https://github.com/mochajs/mocha/pull/3328`
[#&#8203;3330]: `https://github.com/mochajs/mocha/pull/3330`
[#&#8203;3295]: `https://github.com/mochajs/mocha/pull/3295`

[@&#8203;plroebuck]: https://github.com/plroebuck
[@&#8203;harrysarson]: https://github.com/harrysarson
[@&#8203;outsideris]: https://github.com/outsideris
[@&#8203;Munter]: https://github.com/Munter

---

</details>




---

This PR has been generated by [Renovate Bot](https://renovatebot.com).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: usability concerning user experience or interface nice to have enhancement proposal of low priority semver-minor implementation requires increase of "minor" version number; "features"
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants