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

config index fix #1417

Merged
merged 3 commits into from
Nov 17, 2022
Merged

config index fix #1417

merged 3 commits into from
Nov 17, 2022

Conversation

bizob2828
Copy link
Member

  • WIP: reduced complexity for 1 function, fixed all the jsdoc warnings
  • NEWRELIC-5278 refactored parts of lib/config/index.js to be less complex essentially by breaking the functions down further

Proposed Release Notes

Links

Closes NEWRELIC-5278

Details

Extracted pieces of logic into their own functions to make certain functions less complex. I also bumped our ecmaVersion in eslint to 2020 as Node.js 14 is 95% compliant. The only thing it does not support is spread parameters after optional chaining. I think we will be safe but I can dig into specifically not allowing this.

…lex essentially by breaking the functions down further. Updated all jsdoc to be compliant with linter.
@codecov
Copy link

codecov bot commented Nov 16, 2022

Codecov Report

Merging #1417 (29b0db7) into main (0232de1) will increase coverage by 0.02%.
The diff coverage is 95.38%.

@@            Coverage Diff             @@
##             main    #1417      +/-   ##
==========================================
+ Coverage   92.53%   92.56%   +0.02%     
==========================================
  Files         175      175              
  Lines       35610    35671      +61     
  Branches       23       23              
==========================================
+ Hits        32951    33018      +67     
+ Misses       2659     2653       -6     
Flag Coverage Δ
esm-unit-tests-14.x 47.57% <ø> (ø)
esm-unit-tests-16.x 92.14% <ø> (ø)
esm-unit-tests-18.x 92.14% <ø> (ø)
integration-tests-14.x 77.11% <76.15%> (+0.05%) ⬆️
integration-tests-16.x 77.21% <76.15%> (+0.05%) ⬆️
integration-tests-18.x 77.21% <76.15%> (+0.06%) ⬆️
unit-tests-14.x 89.50% <95.38%> (+0.03%) ⬆️
unit-tests-16.x 89.57% <95.38%> (+0.03%) ⬆️
unit-tests-18.x 89.55% <95.38%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
lib/config/index.js 94.61% <95.38%> (+0.53%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@jmartin4563 jmartin4563 self-assigned this Nov 16, 2022
Copy link
Contributor

@jmartin4563 jmartin4563 left a comment

Choose a reason for hiding this comment

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

Small nit about one of the jsdocs. Also, can we log a ticket to bump up the branch coverage here? Seems like we would want tests on some of the defaulting behaviors that currently aren't covered.

lib/config/index.js Show resolved Hide resolved
lib/config/index.js Outdated Show resolved Hide resolved
Co-authored-by: Jessica Lopatta <jlopatta@newrelic.com>
@bizob2828
Copy link
Member Author

Small nit about one of the jsdocs. Also, can we log a ticket to bump up the branch coverage here? Seems like we would want tests on some of the defaulting behaviors that currently aren't covered.

Done: https://issues.newrelic.com/browse/NEWRELIC-5406

jmartin4563
jmartin4563 previously approved these changes Nov 17, 2022
@bizob2828 bizob2828 merged commit 16792fc into newrelic:main Nov 17, 2022
This was referenced Dec 5, 2022
@bizob2828 bizob2828 deleted the config-index-fix branch August 28, 2024 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants