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

use debug package for debug logs #2504

Merged
merged 1 commit into from
Jan 31, 2022
Merged

Conversation

FredKSchott
Copy link
Member

Changes

  • It's hard to remember to do debug logging. And when you do remember to add debug log statements, our codebase currently forces you to pass the logging object everywhere, even down into small helper functions if that's where you want to log log.
  • It's preferred to handle debug logging using a global function, which can be called from everywhere even if the logging object is not present. This makes it easy to add debug log statements without having to change function signatures or worry about passing the logging object around.
  • This is acceptable since debugging is a special case where you are not concerned about appearance, and are just looking for a stream of raw data.
  • BONUS: Our other dependencies use this same debug package, which means you can flip on/off and filter debug logs across our entire dependency tree using the same system: https://github.com/debug-js/debug#environment-variables
  • BONUS 2: It looks pretty.

cc @natemoo-re who I've chatted about wanting to do this with since forever.

Testing

  • N/A (tested manually)

Docs

  • N/A

@changeset-bot
Copy link

changeset-bot bot commented Jan 30, 2022

⚠️ No Changeset found

Latest commit: 38b5b80

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Jan 30, 2022
@FredKSchott FredKSchott force-pushed the debug-logging branch 2 times, most recently from 6bec3ca to cecadda Compare January 30, 2022 23:28
@FredKSchott FredKSchott force-pushed the debug-logging branch 3 times, most recently from 76be48e to e55b3ff Compare January 31, 2022 03:24
@matthewp matthewp merged commit 7e2c4e2 into route-cache-refactor Jan 31, 2022
@matthewp matthewp deleted the debug-logging branch January 31, 2022 19:20
matthewp added a commit that referenced this pull request Jan 31, 2022
* refactor dev to use vite server

* refactor the route cache and other build internals

* use debug package for debug logs (#2504)

Co-authored-by: Matthew Phillips <[email protected]>
@FredKSchott FredKSchott mentioned this pull request Feb 4, 2022
SiriousHunter pushed a commit to SiriousHunter/astro that referenced this pull request Feb 3, 2023
* refactor dev to use vite server

* refactor the route cache and other build internals

* use debug package for debug logs (withastro#2504)

Co-authored-by: Matthew Phillips <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants