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

Fix: config option name not printing as application #128

Conversation

DeanAyalon
Copy link
Contributor

@DeanAyalon DeanAyalon commented Mar 2, 2024

Fixed:

  • config option 'name' not passing to logged data as 'application'
  • Unsupported Logger error message grammer

My apologies, the PR was posted before I had the chance to edit all the information

Tests do not throw errors, except npm run test:package - Which I am not sure about, is it my global Node version crashing it?

Any change needs to be discussed before proceeding. Failure to do so may result in the rejection of the pull request.

Changes:

  1. Uses config name instead of this.name when passing logged data property 'application'
  2. Changes unsupported logger error to logger does not support ${lev} level, also added alternative wording in comment
    • Edits tests/package/configurations - Unsupported logger error expected message accordingly

Fixes #129
closes #129

NPM Tests:

Screen Shot 2024-03-02 at 19 24 59 Screen Shot 2024-03-02 at 19 25 27 Screen Shot 2024-03-02 at 19 25 45 Screen Shot 2024-03-02 at 19 26 16

Copy link

sonarcloud bot commented Mar 2, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@DeanAyalon DeanAyalon changed the title Fix: config option name not printing as application; Fix: config option name not printing as application Mar 2, 2024
@DeanAyalon
Copy link
Contributor Author

Apologies for the mess, Github published my PR before posting the Issue or editing the information within

And sorry for the wait, I had this fix for a while but had other things on my mind 😅

@pustovitDmytro
Copy link
Owner

Hi, looks ok, but it would be better if you cover this with 2 tests(with and without name option) can you add them?

@DeanAyalon
Copy link
Contributor Author

I'm sorry I don't think I understand what to check for and where

@pustovitDmytro pustovitDmytro changed the base branch from master to hotfix/config-name April 5, 2024 15:49
@pustovitDmytro pustovitDmytro merged commit 79f3ce6 into pustovitDmytro:hotfix/config-name Apr 5, 2024
6 checks passed
pustovitDmytro added a commit that referenced this pull request Apr 5, 2024
* Fix: config option name not printing as application; Minor grammar fix for unsupported logger error (#128)

* Chore: adds tests to application name configuration

---------

Co-authored-by: Dean Ayalon <deanayalon@me.com>
pustovitDmytro added a commit that referenced this pull request Apr 5, 2024
…x for unsupported logger error (#130)

* Fix: config option name not printing as application; Minor grammar fix for unsupported logger error (#128)

* Chore: adds tests to application name configuration

---------

Co-authored-by: Dean Ayalon <deanayalon@me.com>
pustovitDmytro pushed a commit that referenced this pull request Apr 5, 2024
## [1.8.1](v1.8.0...v1.8.1) (2024-04-05)

### Chore

* Update devDependencies (non-major) ([15a82eb](15a82eb))

### Fix

* config option name not printing as application; Minor grammar fix for unsupported logger error (#130) ([4e73cd1](4e73cd1)), closes [#130](#130) [#128](#128)
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.

Bug: config name option not working [1.8.0]
2 participants