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

feat(rules): add 'valid-suite-description' rule #74

Merged
merged 4 commits into from
Jul 4, 2016
Merged

feat(rules): add 'valid-suite-description' rule #74

merged 4 commits into from
Jul 4, 2016

Conversation

alecxe
Copy link
Contributor

@alecxe alecxe commented Jul 3, 2016

Should close #69. Thanks.

@coveralls
Copy link

coveralls commented Jul 3, 2016 •
edited
Loading

Coverage Status

Coverage remained the same at 100.0% when pulling 0279dc8 on alecxe:master into 6465f1c on lo1tuma:master.

@alecxe
Copy link
Contributor Author

alecxe commented Jul 3, 2016 •
edited
Loading

Btw, valid-test-description and valid-suite-description rules are quite similar in their implementation. To avoid code duplication we should probably refactor these rules and extract similar code chunks into reusable exported functions. I would though address that after both rules are present in the codebase. What do you think? Thanks.

@lo1tuma
Copy link
Owner

lo1tuma commented Jul 4, 2016

Code LGTM. It seems like there are some merge conflicts. Can you rebase and resolve those conflicts?

Extracting duplicated code into re-useable libs would be very nice. I was also planning to do this for some other rules in this plugin but haven’t found time yet.

@lo1tuma lo1tuma mentioned this pull request Jul 4, 2016
Closed
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling b39dfb8 on alecxe:master into 86e734e on lo1tuma:master.

1 similar comment
@coveralls
Copy link

coveralls commented Jul 4, 2016 •
edited
Loading

Coverage Status

Coverage remained the same at 100.0% when pulling b39dfb8 on alecxe:master into 86e734e on lo1tuma:master.

@alecxe
Copy link
Contributor Author

alecxe commented Jul 4, 2016 •
edited
Loading

Okay, fixed the conflicts. Thanks.

Sorry for the merge commit, should have avoided that.

@lo1tuma
Copy link
Owner

lo1tuma commented Jul 4, 2016

Thank you.

@lo1tuma lo1tuma merged commit d44623e into lo1tuma:master Jul 4, 2016
@lo1tuma lo1tuma added the feature label Jul 4, 2016
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
feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Rule proposal: add "valid-suite-description" rule
3 participants
@alecxe @coveralls @lo1tuma