Skip to content

fix(types): add string to Includeable#11003

Merged
SimonSchick merged 2 commits intosequelize:masterfrom
zjr:master
Jul 5, 2019
Merged

fix(types): add string to Includeable#11003
SimonSchick merged 2 commits intosequelize:masterfrom
zjr:master

Conversation

@zjr
Copy link
Copy Markdown
Contributor

@zjr zjr commented May 25, 2019

This is allowable for Aliases, according to docs and lack of errors when attempting.
See Types: options.include

Pull Request check-list

Please make sure to review and check all of these items:

  • Does npm run test or npm run test-DIALECT pass with this change (including linting)?
  • Does the description below contain a link to an existing issue (Closes #[issue]) or a description of the issue you are solving?
  • Have you added new tests to prevent regressions? (maybe?)
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)? (not sure if types could as docs? kinda?)
  • Did you follow the commit message conventions explained in CONTRIBUTING.md?

Description of change

I tried to follow what this person did at #10804 as far as tests go, but, TBH I didn't look into how these typedef tests work.

This just adds string as a possible type for Includeable as is allowed for Aliases.

@codecov
Copy link
Copy Markdown

codecov bot commented May 25, 2019

Codecov Report

Merging #11003 into master will increase coverage by 4.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #11003      +/-   ##
==========================================
+ Coverage   92.33%   96.34%   +4.01%     
==========================================
  Files          91       94       +3     
  Lines        8727     9013     +286     
==========================================
+ Hits         8058     8684     +626     
+ Misses        669      329     -340
Impacted Files Coverage Δ
lib/dialects/sqlite/index.js 100% <0%> (ø)
lib/dialects/sqlite/query.js 98.64% <0%> (ø)
lib/dialects/sqlite/connection-manager.js 100% <0%> (ø)
lib/data-types.js 91.28% <0%> (+0.34%) ⬆️
lib/dialects/abstract/query-generator.js 97.59% <0%> (+0.71%) ⬆️
lib/dialects/abstract/query.js 91.77% <0%> (+0.98%) ⬆️
lib/sequelize.js 95.91% <0%> (+1.25%) ⬆️
lib/transaction.js 100% <0%> (+2.73%) ⬆️
lib/query-interface.js 92.21% <0%> (+3.4%) ⬆️
lib/errors/database/timeout-error.js 100% <0%> (+60%) ⬆️
... and 3 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 695c87b...49dd431. Read the comment docs.

@zjr
Copy link
Copy Markdown
Contributor Author

zjr commented May 25, 2019

Travis taught me how to test the types and revealed that there was a duplicate alter? definition. This didn't have anything to do with my changes, but I have resolved the issue. Looks like it is causing some failed builds for other PRs too.

@SimonSchick
Copy link
Copy Markdown
Contributor

Pls rebase

@zjr
Copy link
Copy Markdown
Contributor Author

zjr commented Jun 14, 2019 via email

@zjr
Copy link
Copy Markdown
Contributor Author

zjr commented Jun 17, 2019

@SimonSchick I've updated the PR. There's a new failing test since I opened it but I'm fairly certain it has nothing to do with this pull.

@zjr
Copy link
Copy Markdown
Contributor Author

zjr commented Jul 5, 2019

Anything else I should do with this?

@SimonSchick
Copy link
Copy Markdown
Contributor

Rebase 😅

@zjr
Copy link
Copy Markdown
Contributor Author

zjr commented Jul 5, 2019

@SimonSchick rebased

@SimonSchick SimonSchick merged commit ff3c225 into sequelize:master Jul 5, 2019
@sushantdhiman
Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 5.9.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

schmod pushed a commit to schmod/sequelize that referenced this pull request Jul 10, 2019
…ndle-deadlock

* 'master' of github.com:sequelize/sequelize: (22 commits)
  docs(migrations): use timestamps with seed (sequelize#11160)
  test: remove redundant test (sequelize#11156)
  fix(types): add literal to possible where options (sequelize#10990)
  fix(model): don't alter original scopes when combining them (sequelize#10722)
  fix(types): relax order typing (sequelize#10802)
  fix(types): add string to Includeable (sequelize#11003)
  docs(models-definition): correct spelling mistakes (sequelize#11147)
  fix(types): silent option for update (sequelize#11115)
  fix: update sequelize-pool (sequelize#11134)
  feat(hooks): beforeDisconnect / afterDisconnect (sequelize#11117)
  refactor: remove unused _templateSettings
  refactor(query-generation): remove lodash string templates (sequelize#11122)
  docs: improve datatype docs
  docs: explain defaults/where behavior for find/create (sequelize#11069)
  build: remove test*.js from .gitignore (sequelize#11108)
  docs(data-types): extending types
  fix(sequelize.close): update sequelize-pool (sequelize#11101)
  build: update dependencies (sequelize#11099)
  docs(migrations): foreign key example (sequelize#11097)
  fix(mariadb): properly escape json path key (sequelize#11089)
  ...
@sushantdhiman
Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 7.0.0-next.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants