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

Fix Linting for Submitted PR Files #2259

Closed
palisadoes opened this issue Sep 9, 2024 · 22 comments
Closed

Fix Linting for Submitted PR Files #2259

palisadoes opened this issue Sep 9, 2024 · 22 comments
Assignees
Labels
bug Something isn't working feature request

Comments

@palisadoes
Copy link
Contributor

Describe the bug

  1. We want to have consistent coding standards for our repositories.
  2. To do this we added linting to our automated processes.
  3. On certain conditions husky commits and files committed to PRs should fail if the linting fails.
  4. These issues were created to fix this:
    1. Lint git Committed Files & PR Deltas / no-explicit-any #1427
    2. Apply Linting Only to Files Updated in git #1270
    3. Improve and Standardize Automated Linting Tests #861
  5. We need the linting rules to be tightened for each commit and PR

To Reproduce
Steps to reproduce the behavior:

  1. Start the server
  2. Linting messages occur

We don't need the messages on starting the server to be eliminated. We just need to make sure that code committed to the code base in PRs does not have these linting issues. Eventually the linting messages will go away as the code base is improved.

Expected behavior

  1. CLI Husky commits must fail if any of these eslint conditions are present in the committed code:
    1. react/destructuring-assignment
    2. @typescript-eslint/explicit-module-boundary-types
    3. @typescript-eslint/no-explicit-any
    4. @typescript-eslint/no-non-null-assertion
    5. @typescript-eslint/no-unused-vars
  2. PRs must fail if any of these eslint conditions are present in the committed code:
    1. react/destructuring-assignment
    2. @typescript-eslint/explicit-module-boundary-types
    3. @typescript-eslint/no-explicit-any
    4. @typescript-eslint/no-non-null-assertion
    5. @typescript-eslint/no-unused-vars

Actual behavior

  • See above

Screenshots

When starting the Admin server, we get these linting errors.

image

Additional details
Add any other context or screenshots about the feature request here.

Potential internship candidates
Please read this if you are planning to apply for a Palisadoes Foundation internship PalisadoesFoundation/talawa#359

@palisadoes palisadoes added the bug Something isn't working label Sep 9, 2024
Copy link

This issue did not get any activity in the past 10 days and will be closed in 180 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

@github-actions github-actions bot added the no-issue-activity No issue activity label Sep 20, 2024
@Abhinav232004
Copy link
Contributor

@palisadoes ,Could you please assign me this issue?

Copy link

This issue did not get any activity in the past 10 days and will be closed in 180 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

@github-actions github-actions bot added the no-issue-activity No issue activity label Oct 11, 2024
@palisadoes
Copy link
Contributor Author

Unassigning. inactivity

@github-actions github-actions bot removed the no-issue-activity No issue activity label Oct 13, 2024
@Abhinav232004
Copy link
Contributor

Abhinav232004 commented Oct 13, 2024

I was working on it. Could you please assign this issue to me again?

Copy link

This issue did not get any activity in the past 10 days and will be closed in 180 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

@github-actions github-actions bot added the no-issue-activity No issue activity label Oct 24, 2024
@palisadoes
Copy link
Contributor Author

unassigning. inactivity

@github-actions github-actions bot removed the no-issue-activity No issue activity label Oct 28, 2024
@prayanshchh
Copy link
Contributor

I would like to work on this issue

@prayanshchh
Copy link
Contributor

I will start my work on this in 3-4 days as festive season is going on

@prayanshchh
Copy link
Contributor

prayanshchh commented Nov 7, 2024

do i only need to add "@typescript-eslint/no-unused-vars": "error" in .eslintrc.json to solve this issue, as in pull-request.yml we already have:
run: npx eslint ${CHANGED_FILES} && python .github/workflows/eslint_disable_check.py
and in .husky/pre-commit
npm run lint-staged
so we already have checks for linting as well as lint-staged, Is there something more to do here?

@palisadoes
Copy link
Contributor Author

Are all the cases listed in the original issue handled?

@palisadoes
Copy link
Contributor Author

During the week of November 11, 2024 we will start a code freeze on the develop branches in Talawa, Talawa Admin and Talawa-API.

We have completed a project to convert the Talawa-API backend to use PostgreSQL. Work will then begin with us merging code in the develop branches to a new develop-postrgres branch in each repository.

Planning activities for this will be managed in our #talawa-projects slack channel. A GitHub project will be created to track specially labeled issues. We completed a similar exercise last year using a similar methodology.

Starting November 12, California time no new PRs will be accepted against the develop branch. They must be applied to the develop-postrgres branch.

There are some GSoC project features that will need to be merged into develop. These will be the only exceptions.

This activity and the post GSoC 2024 start date was announced in our #general Slack channel last month as a pinned post.

@prayanshchh
Copy link
Contributor

prayanshchh commented Nov 11, 2024

Are all the cases listed in the original issue handled?

except this rule: @typescript-eslint/no-unused-vars, everything is implemented

@palisadoes
Copy link
Contributor Author

Thanks. Please implement that one

@prayanshchh
Copy link
Contributor

alright sir

@prayanshchh
Copy link
Contributor

i am unassigning myself from this one as I need to work on workflow files for postgres in api

@prayanshchh
Copy link
Contributor

would be happy to do this once I am done with api issue

@prayanshchh prayanshchh removed their assignment Nov 17, 2024
@palisadoes
Copy link
Contributor Author

This needs to be done for both the develop and develop-postgres branches.

@prayanshchh
Copy link
Contributor

alright I will do it after the api issue, as I already have two issues assigned

@Suyash878
Copy link
Contributor

Suyash878 commented Nov 22, 2024

Since the required changes have already been merged for the develop branch I would like to implement the necessary changes for the develop-postgres branch.

@palisadoes
Copy link
Contributor Author

Are all the cases listed in the original issue handled?

except this rule: @typescript-eslint/no-unused-vars, everything is implemented

OK. It's assigned to you.

@Suyash878
Copy link
Contributor

I am unassigning myself, since my PR got merged.

@Suyash878 Suyash878 removed their assignment Dec 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working feature request
Projects
None yet
Development

No branches or pull requests

5 participants