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

Bump underscore from 1.12.0 to 1.12.1 #27279

Closed
wants to merge 1 commit into from

Conversation

dependabot[bot]
Copy link
Contributor

@dependabot dependabot bot commented on behalf of github May 29, 2021

Bumps underscore from 1.12.0 to 1.12.1.

Commits

Dependabot compatibility score

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

Dependabot will merge this PR once CI passes on it, as requested by @nextcloud-command.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

Copy link
Member

@nextcloud-bot nextcloud-bot left a comment

Choose a reason for hiding this comment

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

@dependabot merge

@skjnldsv
Copy link
Member

@dependabot rebase

@dependabot dependabot bot force-pushed the dependabot/npm_and_yarn/underscore-1.12.1 branch from 490d981 to 310933c Compare May 29, 2021 13:14
Copy link
Member

@nextcloud-bot nextcloud-bot left a comment

Choose a reason for hiding this comment

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

@dependabot merge

@skjnldsv skjnldsv force-pushed the dependabot/npm_and_yarn/underscore-1.12.1 branch from 310933c to d767c5e Compare May 29, 2021 13:16
@skjnldsv
Copy link
Member

Breaks the tests for some reason 🤔

@juliusknorr juliusknorr added 4. to release Ready to be released and/or waiting for tests to finish 2. developing Work in progress and removed 3. to review Waiting for reviews 4. to release Ready to be released and/or waiting for tests to finish labels Jun 23, 2021
@juliusknorr
Copy link
Member

Looks like jashkenas/underscore#2918

@MichaIng MichaIng added this to the Nextcloud 23 milestone Jul 11, 2021
@dependabot dependabot bot force-pushed the dependabot/npm_and_yarn/underscore-1.12.1 branch from d767c5e to e3165a1 Compare July 11, 2021 17:04
Copy link
Member

@nextcloud-bot nextcloud-bot left a comment

Choose a reason for hiding this comment

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

@dependabot merge

@dependabot dependabot bot force-pushed the dependabot/npm_and_yarn/underscore-1.12.1 branch from e3165a1 to efecef6 Compare July 12, 2021 18:32
Copy link
Member

@nextcloud-bot nextcloud-bot left a comment

Choose a reason for hiding this comment

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

@dependabot merge

@dependabot dependabot bot force-pushed the dependabot/npm_and_yarn/underscore-1.12.1 branch from efecef6 to 180caff Compare July 28, 2021 18:13
@nextcloud-command nextcloud-command force-pushed the dependabot/npm_and_yarn/underscore-1.12.1 branch from 180caff to 422784a Compare July 28, 2021 21:18
@skjnldsv
Copy link
Member

Breaks the tests for some reason

@MichaIng let's pause this upgrade until investigation

#27279 (comment)

@dependabot dependabot bot force-pushed the dependabot/npm_and_yarn/underscore-1.12.1 branch from 422784a to 0e76c98 Compare August 15, 2021 11:07
Copy link
Contributor

@nextcloud-command nextcloud-command left a comment

Choose a reason for hiding this comment

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

@dependabot merge

@nextcloud-command nextcloud-command force-pushed the dependabot/npm_and_yarn/underscore-1.12.1 branch from 0e76c98 to 5d8342d Compare August 15, 2021 14:57
@skjnldsv
Copy link
Member

@MichaIng let's pause this upgrade until investigation

@MichaIng
Copy link
Member

MichaIng commented Sep 2, 2021

I'm trying to understand the underlying issue:

  • The failing tests seem to be about browser window resize events.
  • When the window is resized, some functions are triggered, like additional elements shown or hidden or moved.
  • As during resizing e.g. via non atomic mouse drag, the browser triggers the event very repeatedly, it is not always desired to have the functions being called on each event, but at best only once after the resizing has been completed, to avoid overhead.
  • To achieve this, the actual function calls are wrapped into debounce, which delays them until a certain time after the last debounce call, hence the event must not have been called for a certain time for the functions to be called.
  • In tests, to avoid an unnecessary delay and to avoid trying to check a state while the actual function is still being delayed by debounce, the involved timer is mocked, which basically eliminates the delay, so the actual function is called immediately and the state hence can be checked immediately?
  • And such a mocked timer cannot be used anymore with the changed debounce implementation of underscore 1.12.1.

Taken the above is correct, I see two three solutions:

  1. We do not test whether resizing has the desired effect, but we only test whether calling the actual function(s) has the desired effect. Of course this means that we do not test whether resizing really calls debounce and whether the latter really calls our functions, but probably it is an acceptable compromise, given that the underscore library and its debounce function themselves are tested by their developers.
  2. We add a sleep after the resize event to assure the debounce timer elapsed and the function calls have hence been done, before checking the result. At least for resizing events, I guess we talk about <<1 second, so nothing that would slow down tests in any significant way, given that some take up to 30 minutes, have 120 seconds timeouts for certain state changes etc. This of course demands that we know that the resizing during the test is an atomic step, calling debounce once only, or otherwise that we know for sure the maximum delay it can have during the test.
  3. We check the result in a loop with a timeout, like some drone tests do (though some of them still regularly fail for some reason and delay the tests significantly indeed).

@dependabot @github
Copy link
Contributor Author

dependabot bot commented on behalf of github Dec 18, 2021

A newer version of underscore exists, but since this PR has been edited by someone other than Dependabot I haven't updated it. You'll get a PR for the updated version as normal once this PR is merged.

@MichaIng
Copy link
Member

@dependabot recreate

@nextcloud nextcloud deleted a comment from dependabot bot Dec 18, 2021
@dependabot dependabot bot force-pushed the dependabot/npm_and_yarn/underscore-1.12.1 branch from 5d8342d to aef8244 Compare December 18, 2021 03:17
Copy link
Contributor

@nextcloud-command nextcloud-command left a comment

Choose a reason for hiding this comment

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

@dependabot merge

@dependabot @github
Copy link
Contributor Author

dependabot bot commented on behalf of github Dec 18, 2021

One of your CI runs failed on this pull request, so Dependabot won't merge it.

Dependabot will still automatically merge this pull request if you amend it and your tests pass.

@MichaIng
Copy link
Member

/compile amend /

Bumps [underscore](https://github.com/jashkenas/underscore) from 1.12.0 to 1.12.1.
- [Release notes](https://github.com/jashkenas/underscore/releases)
- [Commits](jashkenas/underscore@1.12.0...1.12.1)

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: npmbuildbot-nextcloud[bot] <npmbuildbot-nextcloud[bot]@users.noreply.github.com>
Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@nextcloud-command nextcloud-command force-pushed the dependabot/npm_and_yarn/underscore-1.12.1 branch from aef8244 to 4b42972 Compare December 18, 2021 14:57
artonge added a commit that referenced this pull request Jan 6, 2022
Reason: #27279
Signed-off-by: Louis Chemineau <louis@chmn.me>
artonge added a commit that referenced this pull request Jan 6, 2022
Reason: #27279
Signed-off-by: Louis Chemineau <louis@chmn.me>
artonge added a commit that referenced this pull request Jan 6, 2022
Reason: #27279
Signed-off-by: Louis Chemineau <louis@chmn.me>
@skjnldsv
Copy link
Member

skjnldsv commented Jan 7, 2022

fixed by #30020

@skjnldsv skjnldsv closed this Jan 7, 2022
@dependabot @github
Copy link
Contributor Author

dependabot bot commented on behalf of github Jan 7, 2022

OK, I won't notify you again about this release, but will get in touch when a new version is available. If you'd rather skip all updates until the next major or minor version, let me know by commenting @dependabot ignore this major version or @dependabot ignore this minor version. You can also ignore all major, minor, or patch releases for a dependency by adding an ignore condition with the desired update_types to your config file.

If you change your mind, just re-open this PR and I'll resolve any conflicts on it.

@dependabot dependabot bot deleted the dependabot/npm_and_yarn/underscore-1.12.1 branch January 7, 2022 07:26
nextcloud-command pushed a commit that referenced this pull request Jan 7, 2022
Reason: #27279
Signed-off-by: Louis Chemineau <louis@chmn.me>
nextcloud-command pushed a commit that referenced this pull request Jan 7, 2022
Reason: #27279
Signed-off-by: Louis Chemineau <louis@chmn.me>
skjnldsv pushed a commit that referenced this pull request Jan 8, 2022
Reason: #27279
Signed-off-by: Louis Chemineau <louis@chmn.me>
@MichaIng MichaIng removed this from the Nextcloud 24 milestone Feb 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants