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.

Sign up for GitHub

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

Jump to bottom

Add check for no argumetn in callback in describes #17

Merged
merged 1 commit into from
Oct 10, 2015
Merged

Add check for no argumetn in callback in describes #17

merged 1 commit into from
Oct 10, 2015

Conversation

SimenB
Copy link
Contributor

@SimenB SimenB commented Oct 9, 2015

Fixes #14

Seems like a low-hanging fruit 😄

I've never written a rule for eslint before, so this might be wrongly implemented

@tlvince
Copy link
Owner

tlvince commented Oct 9, 2015

This is looking good. Thanks Simen.

A few points:

  • It was a little unclear to me going solely by the name (describe-with-done) what the intentions of the rule were. Do you think no-describe-args is clearer? (I thought the comment Enforce that a describe's callback does not contain any arguments was spot on)
  • Could you format the Git commit message as per the AngularJS conventions? This is so a new release will be published automatically by semantic-release. This would be a feat type.
  • Also, per contributing, could you write a brief doc?

If you could address those and squash the commit, that'd be awesome. Thanks for you contributions.

@SimenB
Copy link
Contributor Author

SimenB commented Oct 10, 2015

I went with no-suite-callback-args, is that OK?

I completely forgot about the docs, sorry about that 😆

Should be GTG now.

@SimenB
Copy link
Contributor Author

SimenB commented Oct 10, 2015

@tlvince I added a check for fdescribes and xdescribes as well. fdescribe is an error as well, and while xdescribe wouldn't error, I added it for consistency.

EDIT: And ddescribe for those using Jasmine 1 and karma

@SimenB
feat(rule): add check for no argument in describe's callback
4fd85a4 
This rule asserts that the callback of a suite does
not take any arguments

Fixes #14
SimenB
SimenB reviewed Oct 10, 2015
View reviewed changes
test/rules/no-suite-callback-args.js

var eslintTester = new RuleTester();

eslintTester.run('describe-with-done', rule, {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

might be overkill for the tests, I just added every sample I put in the docs

tlvince added a commit that referenced this pull request Oct 10, 2015
@tlvince
Merge pull request #17 from SimenB/check-arguments-describe
e9de015 
Add check for no argument in callback in describes
@tlvince tlvince merged commit e9de015 into tlvince:master Oct 10, 2015
@tlvince
Copy link
Owner

tlvince commented Oct 10, 2015

Excellent! 👍

@tlvince
Copy link
Owner

tlvince commented Oct 10, 2015

Landed: https://github.com/tlvince/eslint-plugin-jasmine/releases/tag/v1.5.0

@SimenB SimenB deleted the check-arguments-describe branch October 10, 2015 10:31
@mik01aj
Copy link

mik01aj commented Oct 11, 2015

Great! thanks! 👍

@SimenB SimenB mentioned this pull request Feb 3, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@SimenB @tlvince @mik01aj