Skip to content
This repository was archived by the owner on Nov 21, 2025. It is now read-only.

Conversation

@kjin
Copy link
Contributor

@kjin kjin commented May 8, 2019

Prerequisite to addressing a feature request in #994... this change makes every internal config option assignment explicit, and re-organizes internal config options so that instead of interface ParentConfig extends ChildConfig it becomes interface ParentConfig { childConfig: ChildConfig }. This means that each configurable component (internally) only gets config options that it asks for, rather than a large object containing all config options.

Tested behavior should remain the same; some tests have been changed because of the re-organization of internal options.

@kjin kjin requested a review from a team May 8, 2019 01:06
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 8, 2019
@codecov
Copy link

codecov bot commented May 8, 2019

Codecov Report

Merging #1021 into master will decrease coverage by <.01%.
The diff coverage is 96.22%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1021      +/-   ##
==========================================
- Coverage   94.81%   94.81%   -0.01%     
==========================================
  Files          97       96       -1     
  Lines        6150     6186      +36     
  Branches      486      481       -5     
==========================================
+ Hits         5831     5865      +34     
- Misses        165      166       +1     
- Partials      154      155       +1
Impacted Files Coverage Δ
src/trace-plugin-loader.ts 92.51% <ø> (ø) ⬆️
test/plugins/common.ts 95.52% <ø> (ø) ⬆️
test/test-plugin-loader.ts 98% <0%> (ø) ⬆️
test/test-config.ts 94.28% <100%> (ø) ⬆️
src/tracing.ts 88.67% <100%> (-0.21%) ⬇️
src/util.ts 96.9% <100%> (+0.16%) ⬆️
src/trace-api.ts 93.96% <100%> (ø) ⬆️
test/test-util.ts 100% <100%> (ø) ⬆️
test/test-trace-api.ts 100% <100%> (ø) ⬆️
src/index.ts 89.79% <94.73%> (-0.59%) ⬇️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 244633e...d18f153. Read the comment docs.

@codecov
Copy link

codecov bot commented May 8, 2019

Codecov Report

Merging #1021 into master will decrease coverage by <.01%.
The diff coverage is 96.93%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1021      +/-   ##
=========================================
- Coverage   94.81%   94.8%   -0.01%     
=========================================
  Files          97      96       -1     
  Lines        6211    6239      +28     
  Branches      486     483       -3     
=========================================
+ Hits         5889    5915      +26     
- Misses        183     184       +1     
- Partials      139     140       +1
Impacted Files Coverage Δ
src/trace-plugin-loader.ts 92.51% <ø> (ø) ⬆️
test/plugins/common.ts 97.01% <ø> (ø) ⬆️
test/test-plugin-loader.ts 98% <0%> (ø) ⬆️
src/index.ts 96.42% <100%> (+0.27%) ⬆️
test/test-config.ts 95.71% <100%> (ø) ⬆️
src/tracing.ts 88.67% <100%> (-0.21%) ⬇️
src/trace-api.ts 93.96% <100%> (ø) ⬆️
test/test-trace-api.ts 100% <100%> (ø) ⬆️
test/test-config-priority.ts 96% <96%> (-4%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 42a18a5...0a18451. Read the comment docs.

@kjin kjin force-pushed the separate-configs branch from f523b70 to 774952c Compare May 9, 2019 20:23
Copy link
Contributor

@ofrobots ofrobots left a comment

Choose a reason for hiding this comment

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

Almost there.

@kjin kjin force-pushed the separate-configs branch from 774952c to cfc6c41 Compare May 10, 2019 19:39
@kjin kjin merged commit e672b98 into googleapis:master May 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants