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: add insights module #169

Merged
merged 8 commits into from
May 15, 2021
Merged

feat: add insights module #169

merged 8 commits into from
May 15, 2021

Conversation

advaiyalad
Copy link
Contributor

@advaiyalad advaiyalad commented May 8, 2021 •
edited
Loading

Closes #168.

Changes

  • Add insights module
  • Add tests
  • Add docs

Type

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

Comments/notes

Currently, I cannot run tests locally, see #144 (comment). I am opening the PR early so that I can see CI run.

@codecov
Copy link

codecov bot commented May 8, 2021 •
edited
Loading

Codecov Report

Merging #169 (4e9b0e6) into devel (a78dd72) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             devel      #169   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           20        21    +1     
  Lines          334       341    +7     
  Branches        95        96    +1     
=========================================
+ Hits           334       341    +7
Impacted Files Coverage Δ
src/modules/insights.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 a78dd72...4e9b0e6. Read the comment docs.

@advaiyalad advaiyalad marked this pull request as ready for review May 9, 2021 03:10
@gadicc
Copy link
Owner

gadicc commented May 9, 2021

Hey @PythonCreator27, thanks so much, this is fantastic work as usual! Thanks for yet another module!

I'm going to hold off merging a bit just until we solve the other issues, so we can have consistency on the .js extensions and just in case we end up reverting the esm stuff.

@advaiyalad
Copy link
Contributor Author

@gadicc Correct me if I am wrong, but now that the whole tests issue is sorted out, I think that this can be merged. The JSON imports should not matter, as any changes would only affect files that import JSON, which this module does not (directly).

@gadicc
Copy link
Owner

gadicc commented May 14, 2021

Hey @PythonCreator27, you're absolutely right. Sorry, slipped me mind with my upcoming trip. Now that we know we're keeping ESM can I just ask that all the imports are on .js for consistency. I'll tag them inline. Thanks! And of course, thanks again for contributing another awesome module!

gadicc
gadicc requested changes May 14, 2021
View reviewed changes
src/modules/insights.spec.ts Outdated Show resolved Hide resolved
src/other/quoteCombine.spec.ts Show resolved Hide resolved
@advaiyalad
fix: use js extension in tests
4e9b0e6 
Use .js extension in tests and also fix quoteCombine.spec.ts (unintended)
@advaiyalad
Copy link
Contributor Author

Oops. Fixed now. Also generated json http files. @gadicc Anything else?

@advaiyalad advaiyalad closed this May 14, 2021
@advaiyalad advaiyalad reopened this May 14, 2021
@gadicc
Copy link
Owner

gadicc commented May 15, 2021

Awesome, @PythonCreator27, thanks! I think maybe you didn't commit your fix on quoteCombine.spec.ts but it's such an easy fix that I'm going to merge and fix my end to avoid wasting any more time because of the timezone difference between us. One last big final thanks from me for another module!

@gadicc gadicc merged commit 4603232 into gadicc:devel May 15, 2021
gadicc added a commit that referenced this pull request May 15, 2021
@gadicc
chore(tests): add .js to imports in quoteCombine.spec.ts (#169)
7bc60d1
@advaiyalad advaiyalad deleted the feat/addInsightsModule branch May 15, 2021 12:47
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

@gadicc gadicc gadicc requested changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add insights module
2 participants
@advaiyalad @gadicc