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

Feature: source/target keys in belongsToMany associations #11311

Merged
merged 4 commits into from Aug 14, 2019

Conversation

swarthy
Copy link
Contributor

@swarthy swarthy commented Aug 12, 2019

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?
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
  • Did you update the typescript typings accordingly (if applicable)?
  • Did you follow the commit message conventions explained in CONTRIBUTING.md?

Description of change

Closes #4092

@codecov
Copy link

codecov bot commented Aug 12, 2019

Codecov Report

Merging #11311 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #11311      +/-   ##
==========================================
+ Coverage   96.35%   96.35%   +<.01%     
==========================================
  Files          94       94              
  Lines        9071     9089      +18     
==========================================
+ Hits         8740     8758      +18     
  Misses        331      331
Impacted Files Coverage Δ
lib/dialects/abstract/query-generator.js 97.6% <100%> (-0.01%) ⬇️
lib/associations/belongs-to-many.js 98.06% <100%> (+0.13%) ⬆️

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 de06ac3...0990237. Read the comment docs.

@swarthy
Copy link
Contributor Author

swarthy commented Aug 12, 2019

Need some help with test coverage, what I'm doing wrong 😄 ?

Copy link
Member

@papb papb left a comment

Choose a reason for hiding this comment

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

LGTM! Great PR!

Thank you very much for investing some time to help Sequelize!

(Let's see what @sushantdhiman has to say!)

@papb papb added hard For issues and PRs. status: awaiting maintainer type: feature For issues and PRs. For new features. Never breaking changes. labels Aug 13, 2019
@papb
Copy link
Member

papb commented Aug 13, 2019

Need some help with test coverage, what I'm doing wrong smile ?

I don't see anything wrong with the coverage 👍

@sushantdhiman sushantdhiman merged commit 83e263b into sequelize:master Aug 14, 2019
@sushantdhiman
Copy link
Contributor

🎉 This PR is included in version 5.15.0 🎉

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
hard For issues and PRs. released type: feature For issues and PRs. For new features. Never breaking changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Specify non-primary target key in belongsToMany association
3 participants