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.

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

Modularize Server Status JS files #14467

Merged
merged 4 commits into from Jul 28, 2018
Merged

Modularize Server Status JS files #14467

merged 4 commits into from Jul 28, 2018

Conversation

Piyush3079
Copy link
Contributor

This pr contains the modular code for server status files except server_status_queries and server_status_monitor as these two files contains the jQplot library which is not yet included in the code.

This pr is in continuation of the pr Mod_Js_Console. Before reviewing this pr, review Mod_Js_Console and merger and then compare with this pr.

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

@devenbansod devenbansod mentioned this pull request Jul 27, 2018
Merged
4 tasks
@devenbansod devenbansod changed the base branch from master to gsoc-js-refactoring July 27, 2018 15:25
@devenbansod devenbansod changed the title Server Status files modularised Modularize Server Status JS files Jul 27, 2018
@devenbansod devenbansod self-requested a review July 27, 2018 15:25
@devenbansod devenbansod self-assigned this Jul 27, 2018
@devenbansod
Copy link
Member

Please rebase.

@codecov
Copy link

codecov bot commented Jul 27, 2018 •
edited

Codecov Report

❗ No coverage uploaded for pull request base (gsoc-js-refactoring@ff81454). Click here to learn what that means.
The diff coverage is 14.7%.

@@                  Coverage Diff                   @@
##             gsoc-js-refactoring   #14467   +/-   ##
======================================================
  Coverage                       ?   49.93%           
  Complexity                     ?    14435           
======================================================
  Files                          ?      504           
  Lines                          ?    68970           
  Branches                       ?        0           
======================================================
  Hits                           ?    34439           
  Misses                         ?    34531           
  Partials                       ?        0

@Piyush3079 Piyush3079 force-pushed the Mod_Js_Server_Status branch 8 times, most recently from ec28325 to 52a7849 Compare July 27, 2018 16:05
@Piyush3079
Revamp server_status_advisor, server_status_variables, sever_status_p…
a85fb3d 
…rocesses.

Add codemirror package in server_privileges.
Signed-off-by: Piyush Vijay <piyushvijay.1997@gmail.com>
@Piyush3079
Commented code and debug statements removed from server status relate…
30b5f07 
…d files.

Signed-Off-By: Piyush Vijay <piyushvijay.1997@gmail.com>
@Piyush3079
remove .js extension from relevant php files and add those files in a…
b7ee22a 
…jax for dynamic loading.

Signed-Off-By: Piyush Vijay <piyushvijay.1997@gmail.com>
Piyush3079
Piyush3079 commented Jul 28, 2018
View reviewed changes
Copy link
Contributor Author

@Piyush3079 Piyush3079 left a comment

Choose a reason for hiding this comment

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

General comments to increase understandability of the pr

js/src/classes/Server/ProcessList.js
@@ -0,0 +1,164 @@
import { PMA_ajaxShowMessage } from '../../utils/show_ajax_messages';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A class created for handling Server Status Process operations.

js/src/classes/Server/ProcessList.js
// the refresh URL (required to save last used option)
// i.e. full or sorting url
this.refreshUrl = null;
this.init = this.init.bind(this);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bindings done for various functions so that this can be accessed from these functions.

js/src/server_privileges.js
@@ -15,6 +15,7 @@ import { PMA_Messages as PMA_messages } from './variables/export_variables';
import { PMA_ajaxShowMessage, PMA_ajaxRemoveMessage } from './utils/show_ajax_messages';
import { PMA_commonParams } from './variables/common_params';
import { jQuery as $ } from './utils/JqueryExtended';
import { PMA_getSQLEditor } from './utils/sql';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Codemirror library added in modular code and for testing, the code is enabled on server_privileges.php for viewing Privileges or Grant queries dialogue.

js/src/server_status_advisor.js
@@ -0,0 +1,102 @@
/* vim: set expandtab sw=4 ts=4 sts=4: */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modular code for server_status_advisor with imports at the top of file.

js/src/server_status_processes.js
@@ -0,0 +1,41 @@
/* vim: set expandtab sw=4 ts=4 sts=4: */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modular code for server_status_processes with imports at the top of file.

js/src/server_status_variables.js
@@ -0,0 +1,100 @@
/* vim: set expandtab sw=4 ts=4 sts=4: */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modular code for server_status_variables.php with imports at the top of file.

js/src/utils/sql.js
@@ -0,0 +1,272 @@
import CodeMirror from 'codemirror';
import '../../../node_modules/codemirror/mode/sql/sql.js';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Codemirror library imported with addition plugins for linting and hints.

@Piyush3079
Big fix for kill button and autoRefresh button on server_status_proce…
84d0e80 
…sses.php page.

Signed-Off-By: Piyush Vijay <piyushvijay.1997@gmail.com>
devenbansod
devenbansod approved these changes Jul 28, 2018
View reviewed changes
Copy link
Member

@devenbansod devenbansod left a comment

Choose a reason for hiding this comment

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

Looks good to me. Tried running locally works fine.

js/src/utils/sql.js
import '../../../node_modules/codemirror/addon/hint/show-hint.js';
import '../../../node_modules/codemirror/addon/hint/sql-hint.js';
import '../../../node_modules/codemirror/addon/lint/lint.js';
// import '../../../node_modules/codemirror/addon/lint/sql-lint.js';
Copy link
Member

Choose a reason for hiding this comment

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

Is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah this is corrected in the upcoming pr

@devenbansod devenbansod merged commit ed6f106 into phpmyadmin:gsoc-js-refactoring Jul 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@devenbansod devenbansod devenbansod approved these changes

Assignees

@devenbansod devenbansod

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Piyush3079 @devenbansod