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

Linting Process Does Not Fix Certain Issues Automatically #830

Open
RONAK-AI647 opened this issue Nov 19, 2024 · 15 comments
Open

Linting Process Does Not Fix Certain Issues Automatically #830

RONAK-AI647 opened this issue Nov 19, 2024 · 15 comments
Assignees
Labels
category: supporting code Implementation of the system

Comments

@RONAK-AI647
Copy link
Contributor

RONAK-AI647 commented Nov 19, 2024

Lint Tests Passing Locally but Failing in CI/CD Pipeline

What it shows in the terminal-
https://github.com/user-attachments/assets/8761f199-63c8-4e01-b70f-f8d2a77aad4d

What it shows in the CICD pipeline:
Image

Product

kolibri-design-system

Expected behavior

The linter should fix the introduces changes automatically like removing extra spaces ,semicolons etc.

Actual behavior

The linter does not fix the changes automatically , and remains unaltered.

Steps to reproduce the issue

  1. Make a change that violates a linter rule. For example:
    2)Run yarn lint-fix.
    3)Check the output and verify whether the expected issues were fixed.
    4)Run git status to see if any changes were made to the files.

Environment

Local Environment:
Node.js version: 10.x
Yarn version: 1.x
Operating System: Windows

##Comments
This issue has been observed by multiple contributors. Please let me know if additional details or examples are needed.

@RONAK-AI647 RONAK-AI647 changed the title [Title]: Linting Process Does Not Fix Certain Issues Automatically Linting Process Does Not Fix Certain Issues Automatically Nov 19, 2024
@RONAK-AI647
Copy link
Contributor Author

@MisRob I raised the issue , would you put some labels ,if it deserves.

@MisRob
Copy link
Member

MisRob commented Nov 19, 2024

Hi @RONAK-AI647, thank you! I wonder if this could be specific to Windows. I will keep observing with other contributors and perhaps we can see some similarities.

@MisRob MisRob added the category: supporting code Implementation of the system label Nov 19, 2024
@MisRob
Copy link
Member

MisRob commented Nov 19, 2024

Related PR #827

@MisRob
Copy link
Member

MisRob commented Nov 20, 2024

For reference, @rtibbles noted some of the linting issues may be fixed as soon as we upgrade kolibri-tools

@RONAK-AI647
Copy link
Contributor Author

Please Assign it to me @MisRob

@MisRob
Copy link
Member

MisRob commented Dec 13, 2024

Thanks @RONAK-AI647 if you have some new insights, you're welcome to look into this - I noticed you resolved it on one of your PRs

@rtibbles
Copy link
Member

The linting package has now been updated - checking to see if this recurs locally would be helpful. If it is still happening, I think it must be a Windows specific issue.

@MisRob
Copy link
Member

MisRob commented Dec 17, 2024

@RONAK-AI647
Copy link
Contributor Author

I cannot find you in the member's list of learning equality @MisRob ?? I see the contributor tag on ur comments.🧐

@MisRob
Copy link
Member

MisRob commented Dec 17, 2024

Sorry @RONAK-AI647, you should see me there now. It was just something with membership visibility settings.

@RONAK-AI647
Copy link
Contributor Author

Hello maintainers @rtibbles @MisRob , Upon this issue , as many new contributors face this issue of not able to run "yarn lint-fix" command , and the reason I found out was " the jobs of linting( yarn lint-fix is run on UBUNTU machine " which most of the users miss ( running it on the windows machine . If you all allow , the following are my thoughts which can be done or else we would close this issue.

  1. If certain contributors encounter environment-specific issues, you can test linting on multiple environments (e.g., Windows, macOS, and Ubuntu) by adding matrix builds.
    2.Update the dev_docs\01_getting_started.md file , so could it help the new contributors .
  2. Adding a pre-commit hook to run linting automatically before commits. This reduces the chance of contributors missing it.
  3. Otherwise close this issue .

Thank You @AlexVelezLl for guidance .

@RONAK-AI647
Copy link
Contributor Author

The linting package has now been updated - checking to see if this recurs locally would be helpful. If it is still happening, I think it must be a Windows specific issue.

Yes @rtibbles , this is specific to windows latest.

@rtibbles
Copy link
Member

I think adding a pre-commit hook would be the simplest thing here. In our other repositories we use the Python pre-commit package, but I am guessing we've not added it here because no other Python dependencies are involved.

@RONAK-AI647
Copy link
Contributor Author

RONAK-AI647 commented Dec 23, 2024

So if we are not going ahead with Python pre-commit as KDS lack python dependencies , shall we think of node js based pre commit hook system.

@rtibbles
Copy link
Member

Seems like husky is the main solution out there, seems like we could run the same linting command with that, and keep a purely JS ecosystem for KDS.

https://typicode.github.io/husky/how-to.html

Will happily receive a PR that runs yarn run lint-fix with appropriate arguments to ensure it lints only staged files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: supporting code Implementation of the system
Projects
None yet
Development

No branches or pull requests

3 participants