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

refactor: global variables removal #10474

Merged
merged 22 commits into from
Jul 2, 2024

Conversation

4nt0ineB
Copy link
Collaborator

@4nt0ineB 4nt0ineB commented Jun 20, 2024

What

The goal is to get rid of the global variables as suggested by the various 'Todo' in the code, to limit side effects.
These will be accessed through $request_ref :

  • $admin
  • $styles
  • $initjs
  • $cc
  • $bodyabout

About $lc and $countryI'm not sure if I it's a good idea. For a part I can easily apply the same thing as for the other ones, because it's just a matter of putting them in the request_ref (in the .pl cgi). But for the rest, they are used at a very nested level in the call stack. I'd have to add one or two params to many functions to get to them, and where they are called also. It's not unfeasible though.

@github-actions github-actions bot added API Issues related to the Open Food Facts API. More specific labels exist & should be used (API WRITE…) 🗺️ Made Near Me 🕹️ Gamification 🖼️ Images 🧪 tests 📖 Knowledge Panels https://wiki.openfoodfacts.org/Knowledge_panels 🏭 Producers Platform https://wiki.openfoodfacts.org/Platform_for_producers Template::Toolkit The templating toolkit used by product opener. The starting point for HTML/JS/CSS fixes. Display 👥 Users Tags 📨 Mail exports 🧪 unit tests Minion Scanbot file import multilingual products 🌐 Translations 🔎 Search labels Jun 20, 2024
@4nt0ineB 4nt0ineB changed the title Remove global vars fix: Remove global vars Jun 20, 2024
@4nt0ineB 4nt0ineB changed the title fix: Remove global vars refactor: Remove global vars Jun 20, 2024
@4nt0ineB 4nt0ineB changed the title refactor: Remove global vars refactor: global variables removal Jun 21, 2024
@github-actions github-actions bot added API WRITE WRITE API to allow sending product info and image 👮 Moderation labels Jun 24, 2024
@codecov-commenter
Copy link

codecov-commenter commented Jun 24, 2024

Codecov Report

Attention: Patch coverage is 14.28571% with 120 lines in your changes missing coverage. Please review.

Project coverage is 49.30%. Comparing base (dc04d18) to head (fc6dd8b).
Report is 459 commits behind head on main.

Files Patch % Lines
lib/ProductOpener/Display.pm 16.34% 83 Missing and 4 partials ⚠️
lib/ProductOpener/KnowledgePanels.pm 0.00% 11 Missing ⚠️
lib/ProductOpener/Users.pm 0.00% 9 Missing ⚠️
lib/ProductOpener/API.pm 0.00% 6 Missing ⚠️
lib/ProductOpener/Images.pm 0.00% 5 Missing ⚠️
lib/ProductOpener/APITagRead.pm 50.00% 1 Missing ⚠️
lib/ProductOpener/Routing.pm 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10474      +/-   ##
==========================================
- Coverage   49.54%   49.30%   -0.24%     
==========================================
  Files          67       76       +9     
  Lines       20650    21838    +1188     
  Branches     4980     5231     +251     
==========================================
+ Hits        10231    10768     +537     
- Misses       9131     9750     +619     
- Partials     1288     1320      +32     

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

@4nt0ineB 4nt0ineB marked this pull request as ready for review July 1, 2024 14:45
@4nt0ineB 4nt0ineB requested a review from a team as a code owner July 1, 2024 14:45
Copy link
Contributor

@stephanegigandet stephanegigandet 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, thank you!

@github-actions github-actions bot added the 💥 Merge Conflicts 💥 Merge Conflicts label Jul 2, 2024
@github-actions github-actions bot removed the 💥 Merge Conflicts 💥 Merge Conflicts label Jul 2, 2024
@4nt0ineB 4nt0ineB merged commit d9d9a70 into openfoodfacts:main Jul 2, 2024
11 checks passed
@4nt0ineB 4nt0ineB deleted the remove-global-vars branch July 12, 2024 07:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API WRITE WRITE API to allow sending product info and image API Issues related to the Open Food Facts API. More specific labels exist & should be used (API WRITE…) Data import Display exports file import 🕹️ Gamification 🖼️ Images 📖 Knowledge Panels https://wiki.openfoodfacts.org/Knowledge_panels 🗺️ Made Near Me 📨 Mail Minion 👮 Moderation multilingual products 🏭 Producers Platform https://wiki.openfoodfacts.org/Platform_for_producers Scanbot 🔎 Search Tags Template::Toolkit The templating toolkit used by product opener. The starting point for HTML/JS/CSS fixes. 🧪 tests 🌐 Translations 🧪 unit tests 👥 Users
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants