Skip to content

narrow down types for attributes passed to functions like Model.create/Model.build etc #249

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

Merged
merged 1 commit into from
Jan 4, 2018

Conversation

Danlock
Copy link
Contributor

@Danlock Danlock commented Dec 21, 2017

for issue #239
I wanted to typecheck upsert values as well, but it doesn't seem to have a generic overloaded method like all the others.

@codecov-io
Copy link

Codecov Report

Merging #249 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #249   +/-   ##
=======================================
  Coverage   96.71%   96.71%           
=======================================
  Files         115      115           
  Lines        1004     1004           
  Branches      129      129           
=======================================
  Hits          971      971           
  Misses         15       15           
  Partials       18       18

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 634a9ff...92a9ed7. Read the comment docs.

@RobinBuschmann
Copy link
Member

@Danlock Thanks for implementing :) I will look at it very soon!

@RobinBuschmann
Copy link
Member

RobinBuschmann commented Dec 29, 2017

@Danlock Thanks again for your implementation. Works very well. There is one question left from my side: Why do you name the type GetAttributes? Why not ModelAttributes or something like that? :)

@Danlock
Copy link
Contributor Author

Danlock commented Jan 2, 2018

Because rather than simply listing the models attributes, it gets attributes from a model. More of a function than a type really.

@RobinBuschmann
Copy link
Member

@Danlock I would be more happy with ModelAttributes or so. But lets do this later. I will merge it. Thank you very much :)

@RobinBuschmann RobinBuschmann merged commit 1c60b4b into sequelize:master Jan 4, 2018
@Danlock
Copy link
Contributor Author

Danlock commented Jan 7, 2018

Maybe ParsedModelAttributes or FilteredModelAttributes? Thanks for merging.

@RobinBuschmann
Copy link
Member

FilteredModelAttributes sounds good :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants