Skip to content

fix(types): missing upsert hooks#12965

Closed
imsergiobernal wants to merge 3 commits into
sequelize:mainfrom
imsergiobernal:patch-1
Closed

fix(types): missing upsert hooks#12965
imsergiobernal wants to merge 3 commits into
sequelize:mainfrom
imsergiobernal:patch-1

Conversation

@imsergiobernal
Copy link
Copy Markdown
Contributor

@imsergiobernal imsergiobernal commented Jan 21, 2021 •
edited
Loading

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

Upsert hooks are missing in types. I've made this PR directly from Github UI editor.

@imsergiobernal
Missing upsert hooks
ddee74e 
Upsert hooks are missing in types
@imsergiobernal imsergiobernal changed the title Missing upsert hooks fix: missing upsert hooks Jan 21, 2021
@imsergiobernal imsergiobernal changed the title fix: missing upsert hooks fix(types): missing upsert hooks Jan 21, 2021
@imsergiobernal
fix(types): missing upsert hooks
02d0928 
Fixed upsert options type
@imsergiobernal
Copy link
Copy Markdown
Contributor Author

imsergiobernal commented Jan 21, 2021

I want to apply this on V5 as well (our current version). How can we achieve it?

@imsergiobernal
fix(types): missing upsert hooks
6b7f18d 
Just having a bad day
papb
papb requested changes Jun 25, 2021
View reviewed changes
Copy link
Copy Markdown
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.

Hello! I see you are a first-time contributor, thank you for taking the time to help Sequelize! I hope to see more PRs from you in the future!

Please add at least one test to make sure your implementation works as intended and to prevent regressions in the future. For good examples on how to test typescript typings, check the following PRs: 11368, 11379, 11520, 13042. You can also use expect-type if you need more complex assertions for your test.

@papb
Copy link
Copy Markdown
Member

papb commented Jun 25, 2021

I want to apply this on V5 as well (our current version). How can we achieve it?

After this PR is merged, you need to open a new PR pointing to our v5 branch with the same changes (I recommend using git cherry-pick on the merged commit from this PR).

@papb
Copy link
Copy Markdown
Member

papb commented Jun 25, 2021

Also, sorry to take so long to respond here.

@papb papb added status: awaiting response For issues and PRs. OP must respond (or change something, if it is a PR). Maintainers have no action typescript For issues and PRs. Things that involve typescript, such as typings and intellisense. labels Jun 25, 2021
@github-actions github-actions Bot removed the status: awaiting response For issues and PRs. OP must respond (or change something, if it is a PR). Maintainers have no action label Jun 25, 2021
@reediculous456
Copy link
Copy Markdown
Contributor

reediculous456 commented Jun 25, 2021

+1, I ran into this today in my development

@papb papb added the status: awaiting response For issues and PRs. OP must respond (or change something, if it is a PR). Maintainers have no action label Jun 26, 2021
@reediculous456 reediculous456 mentioned this pull request Jul 22, 2021
Merged
6 tasks
@Ygilany
Copy link
Copy Markdown

Ygilany commented Sep 24, 2021

any update on this?

@github-actions github-actions Bot removed the status: awaiting response For issues and PRs. OP must respond (or change something, if it is a PR). Maintainers have no action label Sep 24, 2021
@reediculous456
Copy link
Copy Markdown
Contributor

reediculous456 commented Oct 10, 2021

this was fixed in #13394 and should be closed

@github-actions github-actions Bot added the stale label Oct 27, 2021
@github-actions github-actions Bot closed this Nov 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@papb papb papb requested changes

Assignees

No one assigned

Labels

stale typescript For issues and PRs. Things that involve typescript, such as typings and intellisense.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@imsergiobernal @papb @reediculous456 @Ygilany