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

test: add tests for has_many.js #99

Merged
merged 1 commit into from
Oct 22, 2021
Merged

test: add tests for has_many.js #99

merged 1 commit into from
Oct 22, 2021

Conversation

dulerong
Copy link
Collaborator

test: has_many -> recomputePosition

chore: change web api

test: bindDestroyEvent

test: bindRemoveEvent

test: bindAddEvent

chore: refactored DOM manipulation, deleted useless code

test: bindRemoveEventCallBack

chore: apply new DOM node to tests

test: bindAddEventCallBack

refactor: event call back function

chore: change mock to spy

chore: change mock to spy

chore: lint

test: initSortable new assert

test: bindAddEventCallBack more assertions

chore: delete unnecessary code

refactor: move html elements to separate file

refactor: add html file for test

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behaviour?

Issue Number(s): N/A

Currently does not have test for has_many.js

What is the new behaviour?

  • Added test for has_many.js

Other information

N/A

test: has_many -> recomputePosition

chore: change web api

test: bindDestroyEvent

test: bindRemoveEvent

test: bindAddEvent

chore: refactored DOM manipulation, deleted useless code

test: bindRemoveEventCallBack

chore: apply new DOM node to tests

test: bindAddEventCallBack

refactor: event call back function

chore: change mock to spy

chore: change mock to spy

chore: lint

test: initSortable new assert

test: bindAddEventCallBack more assertions

chore: delete unnecessary code

refactor: move html elements to separate file

refactor: add html file for test
@dulerong
Copy link
Collaborator Author

dulerong commented Oct 21, 2021

@ilunglee
Hey, I decided to open a new branch, the previous branch, for some reason, when I tried rebasing, all the commits I merged from main branch, all scattered into individual commits (commits unrelated). Instead of being grouped in one single commit like "Merge from main branch"

Hence I cherry-picked my commit to a new branch and made a new PR.

I've answered your questions on the older PR, didn't want to force push into it, thought I might delete our conversations (I think it will delete the conversations?).

@ilunglee
Copy link
Contributor

@ilunglee Hey, I decided to open a new branch, the previous branch, for some reason, when I tried rebasing, all the commits I merged from main branch, all scattered into individual commits (commits unrelated). Instead of being grouped in one single commit like "Merge from main branch"

Hence I cherry-picked my commit to a new branch and made a new PR.

I've answered your questions on the older PR, didn't want to force push into it, thought I might delete our conversations (I think it will delete the conversations?).

@dulerong,
I believe the reason why you have an extra commit (f4a9ce6) is because you used git pull --merge (this is the git pull default that runs git fetch and git merge) instead of git pull --rebase (git fetch and git rebase). See the following docs for more information:

Force-pushing won't delete the comments. Please use the same PR (#98), and close this PR.

Thanks!

@dulerong
Copy link
Collaborator Author

@ilunglee Hey, I decided to open a new branch, the previous branch, for some reason, when I tried rebasing, all the commits I merged from main branch, all scattered into individual commits (commits unrelated). Instead of being grouped in one single commit like "Merge from main branch"
Hence I cherry-picked my commit to a new branch and made a new PR.
I've answered your questions on the older PR, didn't want to force push into it, thought I might delete our conversations (I think it will delete the conversations?).

@dulerong, I believe the reason why you have an extra commit (f4a9ce6) is because you used git pull --merge (this is the git pull default that runs git fetch and git merge) instead of git pull --rebase (git fetch and git rebase). See the following docs for more information:

Force-pushing won't delete the comments. Please use the same PR (#98), and close this PR.

Thanks!

@ilunglee
Hey, I've forced push into PR #97, not sure why you mentioned #98 since that's not my original PR.

Will delete this branch later.

@ilunglee
Copy link
Contributor

Thanks! Ooops, I meant #97 😆

@ilunglee ilunglee merged commit 5d0742c into main Oct 22, 2021
@ilunglee ilunglee deleted the jest_test_has_many_new branch October 22, 2021 01:26
@ilunglee
Copy link
Contributor

closed in favour of #97

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.

2 participants