Skip to content
This repository has been archived by the owner on Oct 4, 2023. It is now read-only.

Add lint check for console.log and remove some bad ones #3930

Merged
merged 5 commits into from
Aug 24, 2023

Conversation

raymondjacobson
Copy link
Member

Description

Add lint rule to index.js and update console.log instances to something more relevant.

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide repro instructions & any configuration.

npm run verify

@audius-infra
Copy link
Collaborator

It looks like there may be some changes to native mobile code, which requires triggering a full app release. Please follow the instructions here: https://www.notion.so/audiusproject/When-to-bump-app-version-2644a8f772364a4d91f44abcba44ce0b?pvs=4. cc @nicoback2 @sliptype

Copy link
Contributor

@sliptype sliptype left a comment

Choose a reason for hiding this comment

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

Nice clean up! Prettier won't automatically remove console.logs, for example when debugging right?

@raymondjacobson
Copy link
Member Author

Nice clean up! Prettier won't automatically remove console.logs, for example when debugging right?

nope! will just mark them red

Copy link
Contributor

@dylanjeffers dylanjeffers left a comment

Choose a reason for hiding this comment

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

can you remove the locks?

@raymondjacobson
Copy link
Member Author

can you remove the locks?

done

@pull-request-size pull-request-size bot added size/L and removed size/M labels Aug 23, 2023
@@ -30,6 +30,13 @@ export const initializeSentry = () => {
normalizeDepth: 5,
maxBreadcrumbs: MAX_BREADCRUMBS,
beforeBreadcrumb: (breadCrumb, hint) => {
// filter out info and debug logs
if (
(breadCrumb.level === 'info' || breadCrumb.level === 'debug') &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to filter out info from breadcrumbs?

Copy link
Member Author

Choose a reason for hiding this comment

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

i think so. @dylanjeffers and i were talking about this. info and debug have the same semantic meaning to us internally and there are no real rules of the trade (they show up in console identically...). i think we may just want to only have either debug or info, but left that for future.

Copy link
Contributor

Choose a reason for hiding this comment

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

My brain thinks of them differently:

  • Error: Highest concern/severity
  • Warn: Something unexpected, things might be ok though
  • Info: Useful for debugging prod issues, not noisy
  • Debug: Useful when debugging locally, more permissive to be noisy

I guess my assumption is mainly "debug" => dev, "info" => prod

Maybe I'm unique and other people default to "info" if "log" isn't available, and this methodology would be difficult to get folks to adopt.

I could see a nice use-case of turning debug logs to info logs if we start seeing bugs in prod that we cant repro locally, so that we can debug the issue based on breadcrumbs

@audius-infra
Copy link
Collaborator

It looks like there may be some changes to native mobile code, which requires triggering a full app release. Please follow the instructions here: https://www.notion.so/audiusproject/When-to-bump-app-version-2644a8f772364a4d91f44abcba44ce0b?pvs=4. cc @nicoback2 @sliptype

@audius-infra
Copy link
Collaborator

It looks like there may be some changes to native mobile code, which requires triggering a full app release. Please follow the instructions here: https://www.notion.so/audiusproject/When-to-bump-app-version-2644a8f772364a4d91f44abcba44ce0b?pvs=4. cc @nicoback2 @sliptype

@audius-infra
Copy link
Collaborator

Preview this change https://demo.audius.co/rj-bye-console-log

Copy link
Contributor

@dylanjeffers dylanjeffers left a comment

Choose a reason for hiding this comment

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

noice

@raymondjacobson raymondjacobson merged commit 687fd28 into main Aug 24, 2023
2 checks passed
@raymondjacobson raymondjacobson deleted the rj-bye-console-log branch August 24, 2023 00:13
schottra added a commit that referenced this pull request Aug 25, 2023
* origin/main:
  Add lint check for console.log and remove some bad ones (#3930)
  [C-2968] Fix private collection action buttons (#3937)
  Fix canonical url consistency (#3938)
  [C-2689] Add upload confirmation modal (#3934)
  [C-2966] Make sure that collection description limits are set to 1000 (#3935)
  Move sitemap hostname back to audius.co (#3931)
  Client uses cids in requests to CN for images (#3882)
  Add library albums and playlists audius-query hook + migrate collection reformat util; bump SDK PAY-1679 (#3864)
  [C-2982] Fix seo based on ahref recommendations (#3929)
  Migrate withdraw USDC saga to web common (#3928)
  USDC Withdrawal saga scaffolding (#3926)
  Fix useAllPaginated query C-2980 (#3924)
  Fix infinite scrolling cards C-2979 (#3923)
  [PAY-1632] Clean up and improve performance of music confetti (#3921)
  Revert "Update twitter icon on mobile (#3880)" (#3925)
audius-infra pushed a commit that referenced this pull request Aug 26, 2023
[b39cb5d] [PAY-1733] Remove Gated Prompt Modal (#3948) Marcus Pasell
[a522294] [PAY-1748][PAY-1731][PAY-1729][PAY-1730] DMs link fixes (#3946) Marcus Pasell
[adea357] quick linting fix (#3945) Kyle Shanks
[a0ff27c] Add embed cloudflare deployment and CI (#3940) Raymond Jacobson
[337f80f] [PAY-1727] USDC Withdrawals saga pt. 1 (#3932) Reed
[5bf820c] [C-2956] Add new Access & Sale modal to legacy upload form (#3900) Andrew Mendelsohn
[5258675] [C-2986] Upload flow qa round 1 (#3941) Kyle Shanks
[dd30c09] Use does_current_user_subscribe API field (#3943) Michelle Brier
[778a518] [C-2987] Add UserGeneratedText (#3942) Dylan Jeffers
[bb9e0cf] Update pull_request_template.md (#3939) Raymond Jacobson
[930dd1a] [C-2977] Fix collection page seo (#3936) Dylan Jeffers
[1e50b85] Update README.md (#3911) sabrina-kiam
[687fd28] Add lint check for console.log and remove some bad ones (#3930) Raymond Jacobson
[9f67e7b] [C-2968] Fix private collection action buttons (#3937) Dylan Jeffers
[7ab7a38] Fix canonical url consistency (#3938) Dylan Jeffers
[b1fbef8] [C-2689] Add upload confirmation modal (#3934) Kyle Shanks
[eb41fd3] [C-2966] Make sure that collection description limits are set to 1000 (#3935) Kyle Shanks
[6258484] Move sitemap hostname back to audius.co (#3931) Raymond Jacobson
[1e0c335] Client uses cids in requests to CN for images (#3882) Michelle Brier
[7974acc] Add library albums and playlists audius-query hook + migrate collection reformat util; bump SDK PAY-1679 (#3864) nicoback2
[7cba35b] [C-2982] Fix seo based on ahref recommendations (#3929) Dylan Jeffers
[05744b3] Migrate withdraw USDC saga to web common (#3928) Reed
[5d01710] USDC Withdrawal saga scaffolding (#3926) Reed
[4e0c480] Fix useAllPaginated query C-2980 (#3924) nicoback2
[21eb35c] Fix infinite scrolling cards C-2979 (#3923) nicoback2
[0025261] [PAY-1632] Clean up and improve performance of music confetti (#3921) Raymond Jacobson
[7ed2248] Revert "Update twitter icon on mobile (#3880)" (#3925) Reed
[1091934] [PAY-1742] Remove useMetaMask on invalid account (#3920) Raymond Jacobson
[6e303b1] [PAY-1741] Add routes for transactional pages (#3916) Randy Schott
[74cdb3c] Remove ontouchstart from index.html (#3919) Raymond Jacobson
[1a0332c] Improve lighthouse score (#3918) Raymond Jacobson
[a63896f] [PAY-1706] Merge modalsWithState with modals in common store (#3908) Marcus Pasell
[6a08944] [C-2976] Fix profile-page seo (#3912) Dylan Jeffers
[2381d46] Fix account details css (#3917) Raymond Jacobson
[8c24bdd] Fix mobile share of playlist permalink (#3913) sabrina-kiam
[1e376fe] [C-2911] Update new select page of the upload flow (#3910) Kyle Shanks
[bc0226f] Fix stripe modal opening behavior (#3914) Raymond Jacobson
[a3219c3] [C-2975] Fix stale local data (#3915) Dylan Jeffers
[748fdfd] PAY-1724 Add color specialGreen on mobile (#3909) Reed
[5c59fe5] [PAY-1628] Navigate to track after purchase (#3904) Randy Schott
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants