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

Refactoring: Replace dirname(__FILE__) With Corresponding Constant #2582

Merged
merged 7 commits into from
Sep 9, 2022
Merged

Refactoring: Replace dirname(__FILE__) With Corresponding Constant #2582

merged 7 commits into from
Sep 9, 2022

Conversation

Sdfendor
Copy link
Contributor

@Sdfendor Sdfendor commented Sep 9, 2022

Description (*)

dirname(__FILE__) returns the directory of the current file. Since PHP 5.3.0 the constant __DIR__ exists and is equivalent to the function call (see Magic constants). At the same time it's easier to grasp at first glance. Our current minimum supported PHP version is 7.3.0.
The PR replaces all dirname(__FILE__) calls with DIR except those in lib/Zend and lib/phpseclib which are in the process of being removed from the codebase to become externally maintained dependencies.

Questions or comments

In two instances dirname(dirname(__FILE__)) is replaced with dirname(__DIR__). Another variant here would be dirname(__FILE__),2) (directory name above the current one). I've chosen the first option but have no disposition which one better to use. Maybe some of you have an opinion regarding e.g. ease of understanding?

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)

@github-actions github-actions bot added Component: Bundle Relates to Mage_Bundle Component: Catalog Relates to Mage_Catalog Component: Downloadable Relates to Mage_Downloadable Component: lib/Varien Relates to lib/Varien Component: lib/* Relates to lib/* Component: Reports Relates to Mage_Reports JavaScript Relates to js/* Mage.php Relates to app/Mage.php shell Relates to shell scripts labels Sep 9, 2022
@fballiano
Copy link
Contributor

I like dirname(DIR) because:

  • didn't even know dirname had a second parameter
  • this way we always use DIR instead of mixing DIR and FILE

@Sdfendor
Copy link
Contributor Author

Sdfendor commented Sep 9, 2022

didn't even know dirname had a second parameter

The php documentation states only since 7.0 so it at least wasn't always there 😅.

@sreichel
Copy link
Contributor

sreichel commented Sep 9, 2022

Agree with both, but for me this is more clear ... b/c i see we look one level above ...

dirname(dirname(__FILE__))
dirname(__FILE__, 2)

@fballiano fballiano merged commit 7f56d79 into OpenMage:1.9.4.x Sep 9, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Sep 9, 2022

Unit Test Results

1 files  ±0  1 suites  ±0   0s ⏱️ ±0s
0 tests ±0  0 ✔️ ±0  0 💤 ±0  0 ❌ ±0 
7 runs  ±0  5 ✔️ ±0  2 💤 ±0  0 ❌ ±0 

Results for commit 7f56d79. ± Comparison against base commit ef1b965.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Bundle Relates to Mage_Bundle Component: Catalog Relates to Mage_Catalog Component: Downloadable Relates to Mage_Downloadable Component: lib/Varien Relates to lib/Varien Component: lib/* Relates to lib/* Component: Reports Relates to Mage_Reports JavaScript Relates to js/* Mage.php Relates to app/Mage.php shell Relates to shell scripts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants