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

feat(setGlobalConfig): add setGlobalConfig function #133

Merged
merged 4 commits into from
Apr 24, 2021
Merged

feat(setGlobalConfig): add setGlobalConfig function #133

merged 4 commits into from
Apr 24, 2021

Conversation

advaiyalad
Copy link
Contributor

@advaiyalad advaiyalad commented Apr 12, 2021 •
edited
Loading

Relating to #129.

Changes

  • Add setGlobalConfig function
  • Add tests for setGlobalConfig
  • Add docs
  • Refactor Options interface into a new interfaces.ts file
  • Change schema script to generate schema defs for refactored out interface

Type

  • New Module
  • Other New Feature
  • Validation Fix
  • Other Bugfix
  • Docs
  • Chore/other

Comments/notes

@gadicc Is the name good? Also, all tests seem to pass, yet some unnecessary things have been added to the schema. I am not sure whether it is worth it.
Also, should this function merge config several levels deep, or not?

@codecov
Copy link

codecov bot commented Apr 12, 2021 •
edited
Loading

Codecov Report

Merging #133 (b8cdd1d) into devel (9d86563) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             devel      #133   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           19        20    +1     
  Lines          354       365   +11     
  Branches        93        94    +1     
=========================================
+ Hits           354       365   +11
Impacted Files Coverage Δ
src/lib/options.ts 100.00% <ø> (ø)
src/lib/yahooFinanceFetch.ts 100.00% <ø> (ø)
src/lib/setGlobalConfig.ts 100.00% <100.00%> (ø)

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 9d86563...b8cdd1d. Read the comment docs.

@gadicc gadicc mentioned this pull request Apr 15, 2021
Open
4 tasks
@gadicc
Copy link
Owner

gadicc commented Apr 15, 2021

Hey @PythonCreator27, thanks so much, this is great... before I even had a chance to think about this you did a whole implementation 😅

Yeah I think the extra schema is fine because ultimately we do want to validate this at run time, so it is worth it.

And yes, I think we will need to merge a few levels deep. Ultimately the default config might include quite a lot, and we want the user to be able to override just a single option without needing to copy and paste all the original defaults. So thanks for thinking about that!

Lastly, I edited your brief above to not auto-close #129 because I (right now) added a few other items to the TODO list there.

🙏 🙏

@advaiyalad
fix(setGlobalConfig): merge multiple levels deep
b8cdd1d 
Merges config multiple levels deep. Also fixes docs and tests.
@advaiyalad
Copy link
Contributor Author

@gadicc Now the config is merged multiple levels deep. I think that this is ready for merging. What do you think?
Also, one last note: the anonymous interface has been added again. This is going to be a problem when #145 is merged... Anything that we can do?

@gadicc
Copy link
Owner

gadicc commented Apr 24, 2021

Massive thanks again, @PythonCreator27, this is really great, and full coverage, etc. Merging! 🙏

Re the anonymous interface, let's merge for now and I'll take another look at it when I get a chance.

Thanks again!

@gadicc gadicc merged commit 43ebaa4 into gadicc:devel Apr 24, 2021
@advaiyalad advaiyalad deleted the feat/addSetGlobalConfig branch April 24, 2021 13:23
gadicc pushed a commit that referenced this pull request Jun 1, 2021
@semantic-release-bot
chore(release): 1.11.0 [skip ci]
f6f3e60 
# [1.11.0](v1.10.4...v1.11.0) (2021-06-01)

### Bug Fixes

* **deps:** update dependency ajv to ^8.1.0 ([1769641](1769641))
* **quoteCombine:** resolve `undefined` for missing symbols ([#150](#150)) ([f8c25e3](f8c25e3))
* **testing:** specify jest.js path, not bin ([#170](#170)) ([6772c8e](6772c8e))

### Features

* **options:** accept `date` option ([#186](#186)) ([11b8a72](11b8a72))
* add (friendly) warning when used in the browser ([3c4c5a0](3c4c5a0)), closes [#153](#153)
* add insights module ([#169](#169)) ([4603232](4603232))
* **concurrency:** ability to limit max simultaneous requests ([#76](#76)) ([3424d44](3424d44))
* **modules:** build (true) esm, (interop) cjs modules; tests/readme ([#144](#144)) ([2182f6c](2182f6c))
* **setGlobalConfig:** add setGlobalConfig function ([#133](#133)) ([43ebaa4](43ebaa4))
@gadicc
Copy link
Owner

gadicc commented Jun 1, 2021

🎉 This PR is included in version 1.11.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@gadicc gadicc added the released label Jun 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@advaiyalad @gadicc