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

ADMIN: Fix the eslint_disable_check.py script v2 #3272

Closed
palisadoes opened this issue Jan 14, 2025 · 1 comment
Closed

ADMIN: Fix the eslint_disable_check.py script v2 #3272

palisadoes opened this issue Jan 14, 2025 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@palisadoes
Copy link
Contributor

Describe the bug

  1. Our code base has suffered over the years because contributors ignored linting errors
  2. We created a eslint_disable_check.py script to detect the use of the eslint-disable string in files submitted in the PR
  3. NOTE: The script must fail when:
    1. any of the many variants of eslint-disable are discovered
    2. any future variants of eslint-disable are created
  4. In other words when any of these are discovered at the beginning of any line after stripping white space at the ends and duplicate within multiple white space
    1. // eslint-disable
    2. /* eslint-disable
  5. Your solution must cover any of the options found on the official eslint page
    1. https://eslint.org/docs/latest/use/configure/rules
  6. Take inspiration from the work done here
    1. https://github.com/PalisadoesFoundation/talawa-api/blob/develop-postgres/.github/workflows/scripts/biome_disable_check.py
  7. The previous attempt failed at doing this.
    1. ADMIN: Fix the eslint_disable_check.py script #2257
    2. https://github.com/PalisadoesFoundation/talawa-admin/pull/2318/files#diff-205a96d780b02bbcad4d684e5b75f64d9ac3edb1007ef952630a1fe926ba388b

To Reproduce

Steps to reproduce the behavior:

  1. Submit a PR
  2. Make one of the files have the following line in it (THIS IS AN EXAMPLE OF ONE OF MANY POSSIBLE FAILURE SCENARIOS)
    1. // eslint-disable-next-line @typescript-eslint/no-unused-vars
  3. The PR does not fail

Expected behavior

  1. Submit a PR
  2. Make one of the files have the following line in it
    1. // eslint-disable-next-line @typescript-eslint/no-unused-vars
  3. The PR must fail

Actual behavior

  • See above

Screenshots

  • N/A

Additional details

The same person should be assigned to both these issues

Potential internship candidates

Please read this if you are planning to apply for a Palisadoes Foundation internship

@palisadoes palisadoes added the bug Something isn't working label Jan 14, 2025
@github-actions github-actions bot added duplicate This issue or pull request already exists unapproved labels Jan 14, 2025
@palisadoes palisadoes removed the duplicate This issue or pull request already exists label Jan 14, 2025
@adithyanotfound
Copy link
Contributor

Please assign

palisadoes pushed a commit that referenced this issue Jan 14, 2025
* Fixed eslint-disable script

* Update regex to handle edge cases

* Remove test eslint-disable

* Fixed lint

* Fixed lint

* Fixed redundant \n

* Fixed flake8 lint

* Implemented suggestions

* Removed test eslint-disable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants