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.

Sign up for GitHub

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

Jump to bottom

improvement: Add environment aware linting to prevent console.logs being left in code #1179

Closed
johncowen opened this issue Jul 14, 2023 · 3 comments · Fixed by #2545
Closed

improvement: Add environment aware linting to prevent console.logs being left in code #1179

johncowen opened this issue Jul 14, 2023 · 3 comments · Fixed by #2545
Labels
kind/improvement Improvement on an existing feature triage/accepted The issue was reviewed and is complete enough to start working on it
Milestone
backlog

Comments

@johncowen
Copy link
Contributor

johncowen commented Jul 14, 2023 •
edited
Loading

Description

We want to prevent console.logs from being left in production builds, but we do want to leave console.logs in the code whilst we are working.

We should make two different eslint config files so we can overwrite rules in a production/CI config via a development/local config.

The development config would disable or augment any console log rules (or other rules) in the production config, thus allowing adding console logs only during local development. This development config would then not be used in CI.

Additionally we should configure some git hooks to only run the production rules on commit, meaning they are unlikely to even reach CI. But if for whatever reason the git hook fails or does not get installed, then CI would catch any issues.

Also see #1140
@johncowen johncowen added triage/pending This issue will be looked at on the next triage meeting kind/feature New feature labels Jul 14, 2023
@johncowen johncowen mentioned this issue Jul 14, 2023
Closed
@jakubdyszkiewicz jakubdyszkiewicz added triage/accepted The issue was reviewed and is complete enough to start working on it kind/improvement Improvement on an existing feature and removed triage/pending This issue will be looked at on the next triage meeting kind/feature New feature labels Jul 17, 2023
@github-actions github-actions bot added the triage/stale Inactive for some time. It will be triaged again label Oct 16, 2023
@github-actions
Copy link
Contributor

This issue was inactive for 90 days. It will be reviewed in the next triage meeting and might be closed.
If you think this issue is still relevant, please comment on it or attend the next triage meeting.

@johncowen
Copy link
Contributor Author

Still relevant

@jakubdyszkiewicz jakubdyszkiewicz removed the triage/stale Inactive for some time. It will be triaged again label Nov 9, 2023
@lahabana lahabana added this to the backlog milestone Jan 9, 2024
@github-actions github-actions bot added the triage/stale Inactive for some time. It will be triaged again label Apr 9, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Apr 9, 2024

This issue was inactive for 90 days. It will be reviewed in the next triage meeting and might be closed.
If you think this issue is still relevant, please comment on it or attend the next triage meeting.

@lukidzi lukidzi removed the triage/stale Inactive for some time. It will be triaged again label Apr 10, 2024
@schogges schogges mentioned this issue May 12, 2024
Merged
@johncowen johncowen mentioned this issue May 21, 2024
Merged
johncowen added a commit that referenced this issue May 22, 2024
@johncowen
chore: remove split linting (#2582)
b60c43a 
A while back we decided we wanted a split linting environment (#1179) which we did alongside other improvements in #2545.

Since then we've decided to stick to one single linting environment, i.e. the stricter one. But we still only want to apply fixes on pre-commit and not in CI (CI should fail not fix).

---

I also noticed that:

- We needed `console.info` (you can see usage in  #2531), so aswell as adding that I changed the `console.log`s we used in a cypress config to use `console.info` instead.
- `npm install` was being passed `--frozen-lockfile` which is a `yarn` argument, it should be `npm clean-install`


Signed-off-by: John Cowen <john.cowen@konghq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
kind/improvement Improvement on an existing feature triage/accepted The issue was reviewed and is complete enough to start working on it
Projects
None yet
Milestone
backlog
Development

Successfully merging a pull request may close this issue.

chore: add eslint prod config schogges/kuma-gui
4 participants
@johncowen @lahabana @jakubdyszkiewicz @lukidzi