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

Continuation to #3771 for snake case file naming #3920

Merged
merged 4 commits into from
Jun 29, 2017
Merged

Continuation to #3771 for snake case file naming #3920

merged 4 commits into from
Jun 29, 2017

Conversation

anilreddykatta
Copy link
Contributor

@anilreddykatta anilreddykatta commented Jun 27, 2017

Summary
Conitnuation to #3771. This is WIP. Adding eslint-plugin to enfore this rule. Addresses #3890

Test plan

@codecov-io
Copy link

codecov-io commented Jun 27, 2017

Codecov Report

Merging #3920 into master will not change coverage.
The diff coverage is 8.33%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3920   +/-   ##
=======================================
  Coverage   58.09%   58.09%           
=======================================
  Files         195      195           
  Lines        6751     6751           
  Branches        6        6           
=======================================
  Hits         3922     3922           
  Misses       2826     2826           
  Partials        3        3
Impacted Files Coverage Δ
packages/jest-cli/src/lib/scroll_list.js 100% <ø> (ø)
packages/jest-cli/src/lib/buffered_console.js 7.69% <ø> (ø)
packages/jest-cli/src/generate_empty_coverage.js 87.5% <ø> (ø)
packages/jest-cli/src/reporter_dispatcher.js 41.17% <ø> (ø)
...es/jest-cli/src/lib/handle_deprecation_warnings.js 0% <ø> (ø)
packages/jest-cli/src/lib/prompt.js 95.23% <ø> (ø)
...ackages/jest-cli/src/reporters/summary_reporter.js 22.58% <ø> (ø)
packages/jest-cli/src/reporters/notify_reporter.js 13.04% <ø> (ø)
packages/jest-cli/src/lib/get_max_workers.js 100% <ø> (ø)
packages/jest-cli/src/test_name_pattern_prompt.js 94.11% <ø> (ø)
... and 30 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c8a5135...19fc6bd. Read the comment docs.

@anilreddykatta
Copy link
Contributor Author

@aaronabramov Any inputs for which eslint-plugin to use to enforce this naming convention?

@SimenB
Copy link
Member

SimenB commented Jun 27, 2017

unicorn has it

@anilreddykatta
Copy link
Contributor Author

@SimenB It is running for every file including documentation(MD) files. I don't see any configuration to control this.

@SimenB
Copy link
Member

SimenB commented Jun 27, 2017

Disable it in overrides

@anilreddykatta
Copy link
Contributor Author

@SimenB Thanks..! Let me take a look.

package.json Outdated
@@ -114,5 +114,8 @@
"testMatch": [
"**/*.test.js"
]
},
"dependencies": {
"eslint-plugin-unicorn": "^2.1.2"
Copy link
Member

Choose a reason for hiding this comment

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

dev dep, not dep

Copy link
Contributor Author

@anilreddykatta anilreddykatta Jun 28, 2017

Choose a reason for hiding this comment

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

@SimenB I thought I did it for dev dep..

@anilreddykatta
Copy link
Contributor Author

Need some help. Some of my test cases are failing, even when I don't see any references to any of those fails. I checked out a fresh copy of JEST still same test cases are failing. Is there something I can do about it?
screen shot 2017-06-28 at 2 26 24 pm

@aaronabramov
Copy link
Contributor

@anilreddykatta did you figure it out? it passes locally.
i remember those tests were always flaky. It was something about some nested node_modules directory that was gitignored and hence invisible

@anilreddykatta
Copy link
Contributor Author

anilreddykatta commented Jun 29, 2017

@aaronabramov They were failing locally. I tested on another machine it worked. I added eslint-package to enforce file name convention. I exempted some directories, which are resulting in random test failures. Let's get this one merged so that we have the rule enforced on future file additions. I will work on each exempted directory independently. So, eventually, we will have a consistent naming convention. What do you think?

@aaronabramov
Copy link
Contributor

@anilreddykatta sounds like a plan!
i tested this PR locally, everything seems to work.
Thanks for all the work you're doing @anilreddykatta!

@aaronabramov aaronabramov merged commit e043113 into jestjs:master Jun 29, 2017
@anilreddykatta
Copy link
Contributor Author

@aaronabramov Loving it!

tushardhole pushed a commit to tushardhole/jest that referenced this pull request Aug 21, 2017
* Resolved conflicts

* Including eslint file for forcing file names

* renaming files

* renaming files
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants