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

enforced lint to avoid console.* in prod #187

Merged
merged 10 commits into from
Dec 19, 2024

Conversation

yashpandey06
Copy link
Collaborator

Fixes #186
This PR enforces linting rules to disallow the use of console.* statements in the project. These changes include adding a lint rule to the CI/CD pipeline and updating the package.json to ensure that the rule is checked and enforced during the development and deployment process.

Summary of Changes

  • CI/CD Pipeline:

    • Updated the GitHub Actions CI/CD workflow to run npm run lint as part of the pipeline, ensuring that the console.* lint rule is enforced in every push/merge request.
  • package.json:

    • Ensured the eslint configuration is applied to disallow console.* statements during linting.
    • Added a lint:check script to check for any console.* usage and fail the build if any are found.

@yashpandey06
Copy link
Collaborator Author

@arkid15r please help review these changes .

Copy link
Collaborator

@arkid15r arkid15r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @yashpandey06
Let's discuss some topics below:

.github/workflows/ci-cd.yaml Show resolved Hide resolved
frontend/src/pages/Committees.tsx Outdated Show resolved Hide resolved
frontend/src/pages/Projects.tsx Outdated Show resolved Hide resolved
frontend/package-lock.json Outdated Show resolved Hide resolved
@yashpandey06 yashpandey06 force-pushed the feat/lint-fixes-for-prod branch 2 times, most recently from c3a7b5b to 3282317 Compare December 17, 2024 10:35
frontend/src/utils/logger.ts Show resolved Hide resolved
frontend/src/utils/logger.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@arkid15r arkid15r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the eslint rule for actual no-console enforcement.

frontend/src/utils/logger.ts Show resolved Hide resolved
frontend/package.json Outdated Show resolved Hide resolved
@yashpandey06 yashpandey06 force-pushed the feat/lint-fixes-for-prod branch 3 times, most recently from c33f6ce to 764fba9 Compare December 17, 2024 22:55
@yashpandey06 yashpandey06 force-pushed the feat/lint-fixes-for-prod branch from 764fba9 to f4eb9b9 Compare December 17, 2024 22:58
@yashpandey06 yashpandey06 force-pushed the feat/lint-fixes-for-prod branch from 29a2593 to 1176338 Compare December 17, 2024 23:11
@yashpandey06 yashpandey06 force-pushed the feat/lint-fixes-for-prod branch from 73e26a9 to 32a1bc0 Compare December 17, 2024 23:19
Copy link
Collaborator

@arkid15r arkid15r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks better but I haven't tested it. Please address the recently added comments and let's merge it. I think it's good enough to be merged and improved incrementally.

Thanks for adding this!

frontend/eslint.config.js Outdated Show resolved Hide resolved
frontend/src/sentry.config.ts Outdated Show resolved Hide resolved
frontend/src/utils/logger.ts Outdated Show resolved Hide resolved
frontend/package.json Outdated Show resolved Hide resolved
@yashpandey06 yashpandey06 force-pushed the feat/lint-fixes-for-prod branch from 3b8fc7b to 06daf60 Compare December 18, 2024 09:53
@arkid15r arkid15r enabled auto-merge December 19, 2024 00:03
Copy link
Collaborator

@arkid15r arkid15r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's merge and start using it

@arkid15r arkid15r added this pull request to the merge queue Dec 19, 2024
@arkid15r arkid15r mentioned this pull request Dec 19, 2024
Merged via the queue into OWASP:main with commit 8eafc83 Dec 19, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enforce 'no-console': 'error' eslint rule
2 participants