-
-
Notifications
You must be signed in to change notification settings - Fork 888
fix: globally enable eslint rule #3947
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
fix: globally enable eslint rule #3947
Conversation
WalkthroughThe change updates the ESLint configuration file to modify the file patterns for TypeScript linting. Previously, only TypeScript files in the root directory were targeted. The new configuration applies ESLint rules recursively to all TypeScript files in any subdirectory, ensuring comprehensive linting coverage throughout the project. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant ESLint
participant AllTypeScriptFiles
Developer->>ESLint: Run lint
ESLint->>AllTypeScriptFiles: Apply rules to **/*.ts, **/*.tsx
AllTypeScriptFiles-->>ESLint: Lint results
ESLint-->>Developer: Report issues for all TypeScript files
Assessment against linked issues
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #3947 +/- ##
=================================================
Coverage 87.06% 87.06%
=================================================
Files 362 362
Lines 9315 9315
Branches 1980 1980
=================================================
Hits 8110 8110
Misses 833 833
Partials 372 372 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@palisadoes PTAL. It's ready to get merged. Something is wrong about coderabbit approval workflow check. It's not working as expected. |
@palisadoes |
@disha1202 @noman2002 Can you review this PR ? |
c6102f5
into
PalisadoesFoundation:develop-postgres
I just wanted to point out a few things: this PR definitely fixes the original unused-vars issue, but in addition it’s now triggering forbidden non-null-assertion errors ( Please have a look at Link , I made a commit after the recent changes you did and these eslint errors occurred |
@NishantSinghhhh I opened this issue focusing on unused vars. But the main problem was, every rule was checked only for the root directory. So I made it enabled globally (which must be done) with recursive search. I saw your commit and the errors. Most of them are occuring because the correct convention of naming was not properly followed. It can be easily fixed. Do you want me to help fixing them?
|
Talking about the convention of names, all the names in interface.ts have the word Interface in it which I believe is the conventional way, in December and January The initial commit checks that ran after we made a commit used to throw errors if the interfaces we defined were not having either Interface or I in-front of them , so I belive all names in interface are correctly written , maybe a little change in eslintric.json will remove this specific thing, Also there are few things that I noticed, few errors are not right, here you can see that it shows errors related to that ./CategoryModal' I imported twice, but in the code this is not the case , see the 2nd image, maybe this is not an error from your PR but I guess we need to solve this |
I think your changes passes the eslint check now. However, there were imports from the same file twice (not the same import, but from same file)..but that's fixed now.. |
What kind of change does this PR introduce?
bugfix
Issue Number:
Fixes #3945
Snapshots/Videos:
If relevant, did you update the documentation?
N/A
Summary
Enables eslint rules globally
Does this PR introduce a breaking change?
It prevents memory leaks and other potential issues
Checklist
CodeRabbit AI Review
Test Coverage
Other information
N/A
Have you read the contributing guide?
Yes
Summary by CodeRabbit