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

Add modular code for profiling of sql queries on related pages and structured some code #14518

Merged
merged 4 commits into from Aug 16, 2018

Conversation

Projects
None yet
3 participants
@Piyush3079
Contributor

Piyush3079 commented Jul 24, 2018

This pr contains the functional code for sql profiling. Also this or fixes some of the bugs of modular code.
This pr is in continuation of Mod_Js_Sql_Js. Before reviewing this pr, review and merge Mod_Js_Sql_js.

Before submitting pull request, please check that every commit:

  • Has proper Signed-Off-By
  • Has commit message which describes it
  • Is needed on it's own, if you have just minor fixes to previous commits, you can squash them
  • Any new functionality is covered by tests
@phpmyadmin-bot

This comment has been minimized.

Show comment
Hide comment
@phpmyadmin-bot

phpmyadmin-bot Jul 24, 2018

This pull requests contains too many commits, in most cases it is caused by wrong merge target. In case you have forked master branch you should also ask merging to master branch. See GitHub documentation for more details.

phpmyadmin-bot commented Jul 24, 2018

This pull requests contains too many commits, in most cases it is caused by wrong merge target. In case you have forked master branch you should also ask merging to master branch. See GitHub documentation for more details.

@phpmyadmin-bot

This comment has been minimized.

Show comment
Hide comment
@phpmyadmin-bot

phpmyadmin-bot Jul 24, 2018

This pull requests contains too many commits, in most cases it is caused by wrong merge target. In case you have forked master branch you should also ask merging to master branch. See GitHub documentation for more details.

phpmyadmin-bot commented Jul 24, 2018

This pull requests contains too many commits, in most cases it is caused by wrong merge target. In case you have forked master branch you should also ask merging to master branch. See GitHub documentation for more details.

@devenbansod

This comment has been minimized.

Show comment
Hide comment
@devenbansod

devenbansod Aug 15, 2018

Member

@Piyush3079 Please rebase

Member

devenbansod commented Aug 15, 2018

@Piyush3079 Please rebase

@Piyush3079 Piyush3079 changed the title from Mod_JS Sql Profiling to Add modular code for profiling of sql queries on related pages and structured some code Aug 15, 2018

@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov bot Aug 15, 2018

Codecov Report

Merging #14518 into gsoc-js-refactoring will increase coverage by <.01%.
The diff coverage is 0%.

@@                    Coverage Diff                    @@
##             gsoc-js-refactoring   #14518      +/-   ##
=========================================================
+ Coverage                  50.56%   50.57%   +<.01%     
  Complexity                 14439    14439              
=========================================================
  Files                        501      501              
  Lines                      68002    67990      -12     
=========================================================
  Hits                       34388    34388              
+ Misses                     33614    33602      -12

codecov bot commented Aug 15, 2018

Codecov Report

Merging #14518 into gsoc-js-refactoring will increase coverage by <.01%.
The diff coverage is 0%.

@@                    Coverage Diff                    @@
##             gsoc-js-refactoring   #14518      +/-   ##
=========================================================
+ Coverage                  50.56%   50.57%   +<.01%     
  Complexity                 14439    14439              
=========================================================
  Files                        501      501              
  Lines                      68002    67990      -12     
=========================================================
  Hits                       34388    34388              
+ Misses                     33614    33602      -12

Piyush3079 added some commits May 14, 2018

Profiling working on table browsing and on running sql queries.
Signed-Off-By: Piyush Vijay <piyushvijay.1997@gmail.com>
Bug fix for teardown of page.
Signed-Off-By: Piyush Vijay <piyushvijay.1997@gmail.com>
Add multi column sort for tbl_sql.php and db_sql.php
Signed-Off-By: Piyush Vijay <piyushvijay.1997@gmail.com>
Resolve merge conflicts
Signed-Off-By: Piyush Vijay <piyushvijay.1997@gmail.com>
@devenbansod

Minor comment. Otherwise LGTM

@@ -111,7 +113,7 @@ $(document).ajaxError(function (event, request, settings) {
* Adding common files for every page
*/
for (let i in files.global) {
AJAX.scriptHandler.add(files.global[i]);
AJAX.scriptHandler.add(files.global[i], 1);

This comment has been minimized.

@devenbansod

devenbansod Aug 16, 2018

Member

Why this change?

@devenbansod

devenbansod Aug 16, 2018

Member

Why this change?

This comment has been minimized.

@Piyush3079

Piyush3079 Aug 16, 2018

Contributor

This actually taking care of adding scripts in the _scriptsToBeLoaded in ajax.js. If we don't send 1, the scripts will not be added in the object and on making ajax request the same script will be loaded again without calling teardown for the previous script, causing multiple events for that page.

@Piyush3079

Piyush3079 Aug 16, 2018

Contributor

This actually taking care of adding scripts in the _scriptsToBeLoaded in ajax.js. If we don't send 1, the scripts will not be added in the object and on making ajax request the same script will be loaded again without calling teardown for the previous script, causing multiple events for that page.

@devenbansod devenbansod self-assigned this Aug 16, 2018

@devenbansod devenbansod merged commit 5b1c1a2 into phpmyadmin:gsoc-js-refactoring Aug 16, 2018

1 of 3 checks passed

Codacy/PR Quality Review Codacy was unable to analyse your pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
DCO All commits have a DCO sign-off from the author
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment