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

Enable no-console ESLint rule #39797

Closed
valerybugakov opened this issue Aug 2, 2022 · 4 comments · Fixed by #40458
Closed

Enable no-console ESLint rule #39797

valerybugakov opened this issue Aug 2, 2022 · 4 comments · Fixed by #40458
Assignees
Labels
frontend-platform Issues related to our frontend platform, owned collectively by our frontend crew. gitstart Contract partner

Comments

@valerybugakov
Copy link
Member

valerybugakov commented Aug 2, 2022

Problem statement

Enable no-console ESLint rule

Success criteria

  1. no-console is set to error in the monorepo eslintrc.
  2. no-console is disabled for server-side code (e.g., client/web/dev or gulpfile.js) and Storybook stories via eslintrc overrides field.
  3. no-console is disabled for browser and VSCode packages in their eslintrc configs.
  4. the new logError utility function is added to the @sourcegraph/common package.
    export function logError(...args: unknown[]): void {
        // eslint-disable-next-line no-console
        console.error(...args)
    }
  5. console.error() calls are replaced with the new logError utility function.
  6. console.log() calls are removed or replaced with the new logError utility function.

Time estimate

  • This task should take 3 hours at maximum. Ping the reviewer in the spec pull request if time-consuming changes are required.
@valerybugakov valerybugakov added the gitstart Contract partner label Aug 2, 2022
@valerybugakov valerybugakov added this to To do in GitStart Work Aug 2, 2022
@valerybugakov valerybugakov added the frontend-platform Issues related to our frontend platform, owned collectively by our frontend crew. label Aug 2, 2022
@sourcegraph-bot-2
Copy link
Collaborator

Heads up @taylorsperry @jasongornall - the "team/frontend-platform" label was applied to this issue.

@gitstart-app
Copy link
Contributor

gitstart-app bot commented Aug 2, 2022

Here is the GitStart Ticket for this issue: https://app.gitstart.com/clients/sourcegraph/tickets/SG-39797

@gitstart-sourcegraph gitstart-sourcegraph moved this from To do to In progress in GitStart Work Aug 3, 2022
@gitstart-sourcegraph
Copy link
Collaborator

Hi @valerybugakov
For use cases such as client/shared/dev/suppressPollyErrors.js where we have usage such as console.group / console.groupEnd, and use cases of console.log as shown in the image below, where log message is meant to be informative instead of an error, WDYT about updating the utility function as recommended above, to include non error logs?.

Screenshot 2022-08-08 at 3 28 18 PM

@valerybugakov
Copy link
Member Author

From the issue description:

no-console is disabled for server-side code (e.g., client/web/dev or gulpfile.js) and Storybook stories via eslintrc overrides field.

Hey @gitstart-sourcegraph, for non-application code, we should allow console usage. suppressPollyErrors.js is used to set up integration tests, so keeping these unchanged is fine. Disabling the rule for client/shared/dev is the best way to go here.

@gitstart-sourcegraph gitstart-sourcegraph linked a pull request Aug 22, 2022 that will close this issue
6 tasks
@gitstart-sourcegraph gitstart-sourcegraph moved this from In progress to In spec review in GitStart Work Aug 22, 2022
@gitstart-sourcegraph gitstart-sourcegraph moved this from In spec review to In progress in GitStart Work Sep 27, 2022
GitStart Work automation moved this from In progress to Done Oct 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend-platform Issues related to our frontend platform, owned collectively by our frontend crew. gitstart Contract partner
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants