8000 [WebProfilerBundle] Increase retry delays between toolbarAction ajax calls by JoeKre · Pull Request #25967 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[WebProfilerBundle] Increase retry delays between toolbarAction ajax calls #25967

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

[WebProfilerBundle] Increase retry delays between toolbarAction ajax calls #25967

wants to merge 3 commits into from

Conversation

JoeKre
Copy link
Contributor
@JoeKre JoeKre commented Jan 30, 2018 •
edited
Loading
Q A
Branch? master
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes/no
Fixed tickets #25849
License MIT
Doc PR -

sroze
sroze reviewed Jan 30, 2018
View reviewed changes
src/Symfony/Bundle/WebProfilerBundle/Resources/views/Profiler/toolbar_js.html.twig Outdated
@@ -113,7 +113,7 @@
sfwdt.setAttribute('class', 'sf-toolbar sf-error-toolbar');
}
},
{ maxTries: 5 }
{ maxTries: 5, timeoutDelay: 1000 }
Copy link
Contributor

Choose a reason for hiding this comment

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

timeoutDelay here, while it's retryDelay the other way around? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just fixed it this minute. stupid mistake

stof
stof requested changes Jan 30, 2018
View reviewed changes
src/Symfony/Bundle/WebProfilerBundle/Resources/views/Profiler/base_js.html.twig Outdated
@@ -28,6 +28,7 @@
var xhr = window.XMLHttpRequest ? new XMLHttpRequest() : new ActiveXObject('Microsoft.XMLHTTP');
options = options || {};
options.maxTries = options.maxTries || 0;
options.retryDelay = options.retryDelay || 500;
Copy link
Member

Choose a reason for hiding this comment

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

no need to make this an option IMO. Just change the hardcoded 500 value in the setTimeout call.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Guess that would be f 8000 ine since since the maxTries is only used for loading the toolbar data at least as far as i see it. Therefor i went with extending and keeping the default behaviour unless told otherwise. I'll update the pr

fabpot
fabpot approved these changes Jan 30, 2018
View reviewed changes
Copy link
Member
@fabpot fabpot left a comment

Choose a reason for hiding this comment

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

should be merged on 2.7.

@javiereguiluz javiereguiluz added this to the 2.7 milestone Jan 30, 2018
@javiereguiluz
Copy link
Member

Thanks @JoeKre.

< C3CB /td>

javiereguiluz added a commit that referenced this pull request Jan 30, 2018
@javiereguiluz
minor #25967 [WebProfilerBundle] Increase retry delays between toolba…
b9bb577 
…rAction ajax calls (JoeKre)

This PR was submitted for the master branch but it was squashed and merged into the 2.7 branch instead (closes #25967).

Discussion
----------

[WebProfilerBundle] Increase retry delays between toolbarAction ajax calls

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes/no
| Fixed tickets | #25849
| License       | MIT
| Doc PR        | -

Commits
-------

3288fb0 [WebProfilerBundle] Increase retry delays between toolbarAction ajax calls
@javiereguiluz javiereguiluz mentioned this pull request Jan 30, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sroze sroze sroze left review comments

@fabpot fabpot fabpot approved these changes

@stof stof stof approved these changes

Assignees
No one assigned
Labels
Status: Reviewed WebProfilerBundle
Projects
None yet
Milestone
2.7
Development

Successfully merging this pull request may close these issues.
6 participants
@JoeKre @javiereguiluz @fabpot @stof @sroze @carsonbot
0