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

prepend 'TODO: ' to 'Replace this with your real tests' comments #18828

Merged
merged 3 commits into from
Apr 10, 2020

Conversation

jaydgruber
Copy link
Contributor

@jaydgruber jaydgruber commented Mar 20, 2020

What

  • adds TODO: to the generated Replace this with your real tests comments in test files
  • adds the same to corresponding node-tests fixtures

Why

  • for those in a rush / without strong reviews, you end up with plenty of checked in test files that only have canary tests:
  // Replace this with your real tests.
  test('it exists', function (assert) {
    let service = this.owner.lookup('service:company');
    assert.ok(service);
  });
  • many devs have tooling that highlights TODO or FIXME or similar comments
  • this change will nudge developers to start writing a meaningful test earlier in the process
  • inspired by adding a lint rule here

As far as I can tell, YUIDOC does not have a standard format for TODO comments

JSDOC has @todo

I found a good list of conventions while looking for a highlighter plugin for SublimeText here

@jaydgruber jaydgruber changed the title [DOC] prepend 'TODO: ' to 'Replace this with your real tests' comments WIP: [DOC] prepend 'TODO: ' to 'Replace this with your real tests' comments Mar 21, 2020
@jaydgruber
Copy link
Contributor Author

Looks like I missed a test against a fixture somewhere

@jaydgruber jaydgruber changed the title WIP: [DOC] prepend 'TODO: ' to 'Replace this with your real tests' comments [DOC] prepend 'TODO: ' to 'Replace this with your real tests' comments Mar 22, 2020
@jaydgruber
Copy link
Contributor Author

aha. I missed the entire node-tests directory. that oughta do it

@locks locks changed the title [DOC] prepend 'TODO: ' to 'Replace this with your real tests' comments prepend 'TODO: ' to 'Replace this with your real tests' comments Apr 9, 2020
@rwjblue rwjblue merged commit 286cff2 into emberjs:master Apr 10, 2020
@rwjblue
Copy link
Member

rwjblue commented Apr 10, 2020

Thank you!

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