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

Support nested levelKey #407

Merged
merged 2 commits into from
Mar 13, 2023
Merged

Support nested levelKey #407

merged 2 commits into from
Mar 13, 2023

Conversation

nikrabaev
Copy link
Contributor

@nikrabaev nikrabaev commented Mar 4, 2023

This PR allows specifying the nested levelKey. Such keys will be treated just like the ignore nested keys.
There are two reasons for this change:

  1. Why would the ignore option support nested keys and levelKey don't?
  2. I personally need it because in my project I use the custom format of logs, where the level is placed inside another tags object

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

@nikrabaev
Copy link
Contributor Author

nikrabaev commented Mar 4, 2023

Even though this got approved, I've just realized that it may break things for those who assumed the dot notation handling does not apply to the levelKey and didn't escape the dots in their code.
I see 3 ways to avoid such issues:

  1. Try to find the property using the non-split levelKey first. If the value is not unknown, return it. Otherwise, assume it's a nested key.
  2. Create a new option like nestedLevelKey while preserving the original behavior of levelKey. In situations when both are used, give higher priority to the nestedLevelKey.
  3. Mark it as a breaking change.

@mcollina
Copy link
Member

I think we might just go for a major.

@jsumners wdyt?

Copy link
Member

@jsumners jsumners left a comment

Choose a reason for hiding this comment

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

LGTM with some doc tweaks. I'm fine with a new major.

Readme.md Outdated Show resolved Hide resolved
lib/utils.js Outdated Show resolved Hide resolved
Co-authored-by: James Sumners <james@sumners.email>
@nikrabaev
Copy link
Contributor Author

@jsumners Applied

@mcollina mcollina merged commit 81082ba into pinojs:master Mar 13, 2023
avaitonis added a commit to UK-Export-Finance/mdm-api that referenced this pull request Mar 14, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [pino-pretty](https://togithub.com/pinojs/pino-pretty) | [`^9.4.0` ->
`^10.0.0`](https://renovatebot.com/diffs/npm/pino-pretty/9.4.0/10.0.0) |
[![age](https://badges.renovateapi.com/packages/npm/pino-pretty/10.0.0/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/npm/pino-pretty/10.0.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/npm/pino-pretty/10.0.0/compatibility-slim/9.4.0)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/npm/pino-pretty/10.0.0/confidence-slim/9.4.0)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

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

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

[Compare
Source](https://togithub.com/pinojs/pino-pretty/compare/v9.4.0...v10.0.0)

#### What's Changed

- Bump coverallsapp/github-action from 1.1.3 to 1.2.4 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[pinojs/pino-pretty#405
- Bump tsd from 0.25.0 to 0.26.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[pinojs/pino-pretty#409
- Bump tsd from 0.26.1 to 0.27.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[pinojs/pino-pretty#413
- Support nested levelKey by
[@&#8203;nikrabaev](https://togithub.com/nikrabaev) in
[pinojs/pino-pretty#407

#### New Contributors

- [@&#8203;nikrabaev](https://togithub.com/nikrabaev) made their first
contribution in
[pinojs/pino-pretty#407

**Full Changelog**:
pinojs/pino-pretty@v9.4.0...v10.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 [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/UK-Export-Finance/mdm-api).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xNjAuMCIsInVwZGF0ZWRJblZlciI6IjM0LjE2MC4wIn0=-->
@krazar
Copy link

krazar commented Mar 15, 2023

I'll comment here, but tell me If it is best I create a new issue for this.

This change may introduce an unexpected situation when using a nested level key.
Though the nested key is identified as a level, it is not removed from the object as it is the case with "non nested levelKey" and it could be the intended behavior (at least it is mine).

Using the exclude options could have been a good alternative, but this is applied before loading the level property, so it does not work.

https://github.com/pinojs/pino-pretty/blob/master/index.js#L189

ddadaal pushed a commit to PKUHPC/SCOW that referenced this pull request Mar 20, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [pino-pretty](https://togithub.com/pinojs/pino-pretty) | [`9.4.0` ->
`10.0.0`](https://renovatebot.com/diffs/npm/pino-pretty/9.4.0/10.0.0) |
[![age](https://badges.renovateapi.com/packages/npm/pino-pretty/10.0.0/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/npm/pino-pretty/10.0.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/npm/pino-pretty/10.0.0/compatibility-slim/9.4.0)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/npm/pino-pretty/10.0.0/confidence-slim/9.4.0)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

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

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

[Compare
Source](https://togithub.com/pinojs/pino-pretty/compare/v9.4.0...v10.0.0)

#### What's Changed

- Bump coverallsapp/github-action from 1.1.3 to 1.2.4 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[pinojs/pino-pretty#405
- Bump tsd from 0.25.0 to 0.26.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[pinojs/pino-pretty#409
- Bump tsd from 0.26.1 to 0.27.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[pinojs/pino-pretty#413
- Support nested levelKey by
[@&#8203;nikrabaev](https://togithub.com/nikrabaev) in
[pinojs/pino-pretty#407

#### New Contributors

- [@&#8203;nikrabaev](https://togithub.com/nikrabaev) made their first
contribution in
[pinojs/pino-pretty#407

**Full Changelog**:
pinojs/pino-pretty@v9.4.0...v10.0.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "every weekend" (UTC), 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 these
updates again.

---

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

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/PKUHPC/SCOW).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS4xMC4yIiwidXBkYXRlZEluVmVyIjoiMzUuMTAuMiJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
abhi-markan added a commit to UK-Export-Finance/nestjs-template that referenced this pull request Mar 23, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [pino-pretty](https://togithub.com/pinojs/pino-pretty) | [`^9.4.0` ->
`^10.0.0`](https://renovatebot.com/diffs/npm/pino-pretty/9.4.0/10.0.0) |
[![age](https://badges.renovateapi.com/packages/npm/pino-pretty/10.0.0/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/npm/pino-pretty/10.0.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/npm/pino-pretty/10.0.0/compatibility-slim/9.4.0)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/npm/pino-pretty/10.0.0/confidence-slim/9.4.0)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

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

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

[Compare
Source](https://togithub.com/pinojs/pino-pretty/compare/v9.4.0...v10.0.0)

#### What's Changed

- Bump coverallsapp/github-action from 1.1.3 to 1.2.4 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[pinojs/pino-pretty#405
- Bump tsd from 0.25.0 to 0.26.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[pinojs/pino-pretty#409
- Bump tsd from 0.26.1 to 0.27.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[pinojs/pino-pretty#413
- Support nested levelKey by
[@&#8203;nikrabaev](https://togithub.com/nikrabaev) in
[pinojs/pino-pretty#407

#### New Contributors

- [@&#8203;nikrabaev](https://togithub.com/nikrabaev) made their first
contribution in
[pinojs/pino-pretty#407

**Full Changelog**:
pinojs/pino-pretty@v9.4.0...v10.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 [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/UK-Export-Finance/nestjs-template).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xNjAuMCIsInVwZGF0ZWRJblZlciI6IjM0LjE2MC4wIn0=-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants