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

fix: unescape \\ for levelKey #538

Merged
merged 2 commits into from
Nov 8, 2024
Merged

fix: unescape \\ for levelKey #538

merged 2 commits into from
Nov 8, 2024

Conversation

eliw00d
Copy link
Contributor

@eliw00d eliw00d commented Nov 4, 2024

closes #456

eliw00d
eliw00d commented Nov 4, 2024
View reviewed changes
lib/pretty.js
typeof log[key] === 'boolean'
})
]
.map((key) => key.replaceAll(/\\/g, ''))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This strips off the \\ in log\\.level so that you get the correct key to ignore.

eliw00d
eliw00d commented Nov 4, 2024
View reviewed changes
test/basic.test.js
const formatted = pretty(chunk.toString())
t.equal(
formatted,
`[${formattedEpoch}] WARN (${pid}): foo\n`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without this fix, you would get log.level underneath in the prettified object.

mcollina
mcollina approved these changes Nov 5, 2024
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@eliw00d
Copy link
Contributor Author

eliw00d commented Nov 5, 2024 •
edited
Loading

@mcollina Pino 9.0.0 dropped support for Node 14. Is that something that can be done here as well? I can switch to replace if not.

@jsumners
Copy link
Member

jsumners commented Nov 5, 2024

Yes, we should drop 14. It'll be a new major, though.

@eliw00d
Copy link
Contributor Author

eliw00d commented Nov 5, 2024

Yes, we should drop 14. It'll be a new major, though.

Should I make that change in this PR? For example:

      matrix:
        node-version: [18.17, 20, 22]
        os: [ubuntu-latest]
        pino-version: [^9.0.0]

@mcollina
Copy link
Member

mcollina commented Nov 6, 2024

@eliw00d send a fresh PR.

@eliw00d
Copy link
Contributor Author

eliw00d commented Nov 6, 2024

@eliw00d send a fresh PR.

Done.

@eliw00d eliw00d requested a review from mcollina November 7, 2024 15:02
@eliw00d
Copy link
Contributor Author

eliw00d commented Nov 7, 2024

@jsumners @mcollina Should be good to go now! 🎉

mcollina
mcollina approved these changes Nov 7, 2024
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina merged commit 2a85c0d into pinojs:master Nov 8, 2024
5 checks passed
@eliw00d eliw00d deleted the fix/levelKey branch November 8, 2024 16:22
alexandresoro pushed a commit to alexandresoro/ouca that referenced this pull request Nov 26, 2024
@alexandresoro
fix(deps): update dependency pino-pretty to v13 (#308)
2311830 
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [pino-pretty](https://github.com/pinojs/pino-pretty) | dependencies | major | [`11.3.0` -> `13.0.0`](https://renovatebot.com/diffs/npm/pino-pretty/11.3.0/13.0.0) |

---

### Release Notes

<details>
<summary>pinojs/pino-pretty (pino-pretty)</summary>

### [`v13.0.0`](https://github.com/pinojs/pino-pretty/releases/tag/v13.0.0)

[Compare Source](pinojs/pino-pretty@v12.1.0...v13.0.0)

#### What's Changed

-   chore: replace readable-stream with built-in stream by [@&#8203;benmccann](https://github.com/benmccann) in pinojs/pino-pretty#542

#### New Contributors

-   [@&#8203;benmccann](https://github.com/benmccann) made their first contribution in pinojs/pino-pretty#542

**Full Changelog**: pinojs/pino-pretty@v12.1.0...v13.0.0

### [`v12.1.0`](https://github.com/pinojs/pino-pretty/releases/tag/v12.1.0)

[Compare Source](pinojs/pino-pretty@v12.0.0...v12.1.0)

#### What's Changed

-   fix: unescape \ for levelKey by [@&#8203;eliw00d](https://github.com/eliw00d) in pinojs/pino-pretty#538

**Full Changelog**: pinojs/pino-pretty@v12.0.0...v12.1.0

### [`v12.0.0`](https://github.com/pinojs/pino-pretty/releases/tag/v12.0.0)

[Compare Source](pinojs/pino-pretty@v11.3.0...v12.0.0)

#### What's Changed

-   build(deps-dev): lock typescript minor version by [@&#8203;Fdawgs](https://github.com/Fdawgs) in pinojs/pino-pretty#534
-   Bump typescript from 5.0.4 to 5.6.3 by [@&#8203;dependabot](https://github.com/dependabot) in pinojs/pino-pretty#535
-   Drop support for Node.js 14 & 16 by [@&#8203;eliw00d](https://github.com/eliw00d) in pinojs/pino-pretty#540

#### New Contributors

-   [@&#8203;eliw00d](https://github.com/eliw00d) made their first contribution in pinojs/pino-pretty#540

**Full Changelog**: pinojs/pino-pretty@v11.3.0...v12.0.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC4xNDIuNyIsInVwZGF0ZWRJblZlciI6IjM4LjE0Mi43IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJkZXBlbmRlbmNpZXMiXX0=-->

Reviewed-on: https://git.tristess.app/alexandresoro/ouca/pulls/308
Reviewed-by: Alexandre Soro <code@soro.dev>
Co-authored-by: renovate <renovate@git.tristess.app>
Co-committed-by: renovate <renovate@git.tristess.app>
@ademarsj ademarsj mentioned this pull request Dec 16, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jsumners jsumners jsumners approved these changes

@mcollina mcollina mcollina approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Regression with levelKey handling in version 10
3 participants
@eliw00d @jsumners @mcollina