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

Remove global variables #19503

Merged
merged 63 commits into from
Jan 11, 2025
Merged

Remove global variables #19503

merged 63 commits into from
Jan 11, 2025

Conversation

MauricioFauth
Copy link
Member

@MauricioFauth MauricioFauth commented Jan 8, 2025 •
edited
Loading

Removes almost all global variables and replaces them with static properties.

@MauricioFauth
Remove username global variable
a01fed1 
Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove hostname global variable
d253986 
Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the pred_username global variable
c481ab3 
Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the pred_hostname global variable
64f86ad 
Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the new_username global variable
b4cd510 
Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the reload global variable
ebea0aa 
Replace it with ResponseRenderer::$reload static property.

Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the asfile global variable
aad57a8 
Replaces it with Export::$asFile static property.

Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the SESSION_KEY global variable
a164696 
Replaces it with the Import\Ajax::SESSION_KEY constant.

Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the import_text global variable
f39f285 
Replaces it with Import::$importText static property.

Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the save_on_server global variable
0958b97 
Replaces it with Export::$saveOnServer static property.

Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the output_charset_conversion global variable
716c4bb 
Replaces it with Export::$outputCharsetConversion static property.

Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the buffer_needed global variable
b3df04b 
Replaces it with Export::$bufferNeeded static property.

Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the output_kanji_conversion global variable
ae305d6 
Replaces it with Export::$outputKanjiConversion static property.

Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the where_clause global variable
11766ad 
Replaces it with Current::$whereClause static property.

Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove some unused global variables
1998aa7 
Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the single_table global variable
138c502 
Replaces it with Export::$singleTable static property.

Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the conn_error global variable
1406816 
Replaces it with AuthenticationCookie::$connectionError static property.

Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@codecov Codecov
Copy link

codecov bot commented Jan 8, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 46.61922% with 150 lines in your changes missing coverage. Please review.

Project coverage is 60.13%. Comparing base (91fe474) to head (99c70e3).
Report is 36 commits behind head on master.

Files with missing lines Patch % Lines
src/Controllers/Server/PrivilegesController.php 33.33% 24 Missing ⚠️
src/Export/Export.php 13.63% 19 Missing ⚠️
src/Controllers/Import/ImportController.php 37.03% 17 Missing ⚠️
src/Controllers/Export/ExportController.php 80.95% 8 Missing ⚠️
src/Import/Import.php 20.00% 8 Missing ⚠️
src/Plugins/Auth/AuthenticationCookie.php 76.19% 5 Missing ⚠️
src/Controllers/Database/ExportController.php 0.00% 4 Missing ⚠️
...rollers/Database/Structure/DropTableController.php 0.00% 4 Missing ⚠️
src/Controllers/Server/ExportController.php 0.00% 4 Missing ⚠️
src/Import/Ajax.php 0.00% 4 Missing ⚠️
... and 25 more
Additional details and impacted files 
@@            Coverage Diff            @@
##             master   #19503   +/-   ##
=========================================
  Coverage     60.12%   60.13%           
+ Complexity    15881    15873    -8     
=========================================
  Files           676      675    -1     
  Lines         61188    61159   -29     
=========================================
- Hits          36791    36778   -13     
+ Misses        24397    24381   -16
Flag Coverage Δ
dbase-extension ?
unit-8.2-ubuntu-latest ?
unit-8.3-ubuntu-latest ?
unit-8.4-ubuntu-latest ?
unit-8.5-ubuntu-latest 60.13% <46.61%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MauricioFauth
Remove the display_query global variable
0669a17 
Replaces it with Current::$displayQuery static property.

Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the errno global variable
f5d99ce 
Replaces it with DatabaseInterface::$errorNumber static property.

Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the charset global variable
b962abf 
Replaces it with Current::$charset static property.

Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the pma_auth_server global variable
aa9736b 
Replaces it with AuthenticationCookie::$authServer static property.

Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the cached_affected_rows global variable
7742e3a 
Replaces it with a property.

Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the complete_query global variable
17371f0 
Replaces it with Current::$completeQuery static property.

Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the compression global variable
0ffbe20 
Replaces it with Export::$compression static property.

Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the file_handle global variable
4f4f31e 
Replaces it with Export::$fileHandle static property.

Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the from_cookie global variable
06c62ee 
Replaces it with AuthenticationCookie::$fromCookie static property.

Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the knjenc global variable
941a1e2 
Replaces it with Export::$kanjiEncoding static property.

Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the maxsize global variable
01a9449 
Replaces it with Export::$maxSize static property.

Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the memory_limit global variable
04f1bef 
Replaces it with Export::$memoryLimit static property.

Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the importHandle global variable
1176de2 
Replaces it with a static property.

Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the buffer global variable
43cd4f4 
Replaces it with a static property.

Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the eof global variable
25e0ef6 
Replaces it with ImportShp::$eof static property.

Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the special_message global variable
c635322 
Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the buffer_message global variable
c2cdfd7 
Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the exporting_metadata global variable
728f682 
Replaces it with a static property.

Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Remove the current_result global variable
657d1d0 
Replaces it with a local variable.

Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@MauricioFauth
Access _POST and _SESSION directly
fa744d7 
Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@phpmyadmin-bot
Copy link
Contributor

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.

1 similar comment
@phpmyadmin-bot
Copy link
Contributor

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.

@MauricioFauth MauricioFauth changed the title Remove some global variables Remove global variables Jan 10, 2025
@phpmyadmin-bot
Copy link
Contributor

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.

@MauricioFauth MauricioFauth marked this pull request as ready for review January 10, 2025 21:31
@phpmyadmin-bot
Copy link
Contributor

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.

kamil-tekiela
kamil-tekiela approved these changes Jan 10, 2025
View reviewed changes
Copy link
Contributor

@kamil-tekiela kamil-tekiela left a comment

Choose a reason for hiding this comment

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

I agree with the core concept, but I am not sure if I want to review it in detail. My web browser is lagging. The test is failing though.

williamdes
williamdes approved these changes Jan 10, 2025
View reviewed changes
Copy link
Member

@williamdes williamdes left a comment

Choose a reason for hiding this comment

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

Do we have no more globals after this PR ?

@MauricioFauth
Fix failing tests
3e5934e 
Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@phpmyadmin-bot
Copy link
Contributor

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.

@MauricioFauth
Copy link
Member Author

I agree with the core concept, but I am not sure if I want to review it in detail. My web browser is lagging. The test is failing though.

Sorry about this huge pull request. I hyper-focused on this and it end up bigger than I expected.
I basically changed as little as possible to not change behavior. So there's some static properties that don't make much sense and need a refactor. But it's easier to catch these cases now.

Do we have no more globals after this PR ?

There are only a few globals now. That should be removed in another PR.

@MauricioFauth
Remove $GLOBALS clean up from AbstractTestCase
ed9dc6c 
There's no need for it anymore.

Signed-off-by: Maurício Meneghini Fauth <mauricio@mfauth.net>
@phpmyadmin-bot
Copy link
Contributor

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.

@MauricioFauth MauricioFauth merged commit 605e56b into phpmyadmin:master Jan 11, 2025
42 checks passed
@MauricioFauth MauricioFauth deleted the globals-removal branch January 11, 2025 19:00
@MauricioFauth MauricioFauth self-assigned this Jan 11, 2025
@phpmyadmin-bot
Copy link
Contributor

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.

@MauricioFauth MauricioFauth added this to the 6.0.0 milestone Jan 11, 2025
@phpmyadmin-bot
Copy link
Contributor

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.

@williamdes
Copy link
Member

PS: I have a work in progress to fix the bot, will try to finish it. So it stops commenting again and again

@williamdes williamdes mentioned this pull request Jan 13, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@williamdes williamdes williamdes approved these changes

@kamil-tekiela kamil-tekiela kamil-tekiela approved these changes

Assignees

@MauricioFauth MauricioFauth

Labels
None yet
Projects
None yet
Milestone
6.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@MauricioFauth @phpmyadmin-bot @williamdes @kamil-tekiela