-
-
Notifications
You must be signed in to change notification settings - Fork 749
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
Fixed the eslint_disable_check.py script #2521
Fixed the eslint_disable_check.py script #2521
Conversation
WalkthroughThe changes introduce a Python script, Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 ProcessWe have these basic policies to make the approval process smoother for our volunteer team. Testing Your CodePlease make sure your code passes all tests. Our test code coverage system will fail if these conditions occur:
The process helps maintain the overall reliability of the code base and is a prerequisite for getting your PR approved. Assigned reviewers regularly review the PR queue and tend to focus on PRs that are passing. 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 #2521 +/- ##
===========================================
+ Coverage 98.89% 99.09% +0.20%
===========================================
Files 349 349
Lines 17769 17769
Branches 2371 2371
===========================================
+ Hits 17573 17609 +36
+ Misses 193 160 -33
+ Partials 3 0 -3 ☔ View full report in Codecov by Sentry. |
I am not sure but hope this fixed the issue . If any changes required do let me know |
@ARYANSHAH1567 Can you test using the guidance in the issue to make sure it does? |
When i run the .py script individually it returns error that //next-line comment is present but when i run the lint:check it returns all the eslint error. So that's why i am not sure if its working or no |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
setup.ts (1)
222-223
: Use theerror
parameter for logging or handling, or remove it if not needed.It's a good practice to include the
error
parameter in the catch block for capturing and potentially logging the error object for more informative error handling.However, in the current implementation, the
error
parameter is unused, and a comment has been added to disable the ESLint rule for unused variables.If the intention is to log or handle the error in some way, please use the
error
parameter accordingly. Otherwise, consider removing theerror
parameter along with the comment to avoid disabling the ESLint rule unnecessarily.
@palisadoes found the problem. In the pull-request.yml while running the linting error it was running only the eslint check and not executing the eslint-disable.py file due to which these errors were not failing the tests. Now my pr fails for linting due to comment //eslint-disable |
Same for the Admin it also executes only the command npx eslint does not include the eslint-disable.py script |
Have you tried asking our slack channel for assistance? |
I have added the script for disable comment check. Now if I remove the comments from my pr then the pr will pass all its tests |
Once i remove the eslint-disable comment it passes the tests
my previous commit on this pr failed as the line //eslint-disable next line was present |
df7a52d
into
PalisadoesFoundation:develop
What kind of change does this PR introduce?
Bugfix
Issue Number:
Fixes #2519
Summary by CodeRabbit
New Features
eslint-disable
statements in TypeScript files.Bug Fixes
wipeExistingData
function for better context in error logging.