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

feat(eslint): add linting rules for test describe depth #3056

Merged
merged 1 commit into from
Dec 19, 2024

Conversation

gracetxgao
Copy link
Contributor

@gracetxgao gracetxgao commented Sep 11, 2024

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Fixes: #122

What is the new behavior?

Maximum describe() depth of 3 set for tests.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@gracetxgao gracetxgao requested a review from a team as a code owner September 11, 2024 01:56
@damienwebdev
Copy link
Member

This is awesome! @griest024 do you think we should set depth to 4 as warning, and 5 as error? I only see a few with 5, but many more with 3/4.

.eslintrc.js Outdated Show resolved Hide resolved
@griest024
Copy link
Member

@damienwebdev 5 as an error sounds good. Can we mix error and warning values for the same rule? Afaik eslint does not allow for this.

@damienwebdev
Copy link
Member

"eslint-plugin-jasmine": "0.0.0-PLACEHOLDER",
needs an update for the dependency as well

damienwebdev
damienwebdev previously approved these changes Sep 17, 2024
Copy link
Member

@damienwebdev damienwebdev left a comment

Choose a reason for hiding this comment

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

lgtm. Needs @griest024 approval for merge.

@damienwebdev damienwebdev changed the title feat(eslint): add linting rules for testing feat(eslint): add linting rules for test describe depth Sep 17, 2024
tools/eslint/config/index.js Outdated Show resolved Hide resolved
griest024
griest024 previously approved these changes Sep 17, 2024
Copy link
Member

@griest024 griest024 left a comment

Choose a reason for hiding this comment

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

lgtm

@griest024
Copy link
Member

needs a rebase

@damienwebdev
Copy link
Member

@gracetxgao Needs another rebase.

Copy link
Member

@damienwebdev damienwebdev left a comment

Choose a reason for hiding this comment

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

lgtm

@damienwebdev damienwebdev merged commit 1f1f854 into graycoreio:develop Dec 19, 2024
8 checks passed
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.

[FEAT] Linting rules for describe() depth in tests
3 participants