Page MenuHomePhabricator

Simplify the use of `phutil_get_library_root`
ClosedPublic

Authored by joshuaspence on Jan 11 2015, 10:50 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, May 7, 8:03 AM
Unknown Object (File)
Tue, May 7, 7:23 AM
Unknown Object (File)
Fri, May 3, 6:06 AM
Unknown Object (File)
Thu, Apr 25, 1:01 AM
Unknown Object (File)
Wed, Apr 17, 5:32 AM
Unknown Object (File)
Mar 28 2024, 3:01 PM
Unknown Object (File)
Mar 25 2024, 11:17 AM
Unknown Object (File)
Feb 17 2024, 9:25 PM
Subscribers

Details

Summary

Currently, in order to retrieve the library root of the current phutil module, the following code is required:

$library = phutil_get_current_library_name();
$root = phutil_get_library_root($library);

This can be simplified by allowing the use of phutil_get_library_root() (without any arguments) to mean "get the library root for the current module".

Test Plan

N/A

Diff Detail

Repository
rPHU libphutil
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

joshuaspence retitled this revision from to Simplify the use of `phutil_get_library_root`.
joshuaspence updated this object.
joshuaspence edited the test plan for this revision. (Show Details)
joshuaspence added a reviewer: epriestley.
joshuaspence edited edge metadata.

Use this change to simplify some other code

epriestley edited edge metadata.

I'm very slightly concerned that this may break silently when code is moved between libraries. Although this is rare, we've moved a decent amount of code from phabricator to libphutil in the past, and some between libphutil and arcanist, and a bit between arcanist and phabricator. Probably none of it actually called phutil_get_library_root(), but the scenario (where we move code that does) doesn't seem totally impossible to me.

If we're explicit with library names, the code will continue functioning correctly (or throw a very visible/obvious error, in the case of moving to a lower level library with the higher-level library not loaded). With implicit names, it may silently change the behavior of the code.

If this seems like a net positive even given that consideration, I'm fine with it. It seems pretty borderline to me (although I could also imagine library authors having a much easier time with this version of the function in the future, and facing nearly zero risk of cross-library code moves).

This revision is now accepted and ready to land.Jan 11 2015, 5:18 PM

I think that this change makes phutil_get_library_root() more useful, although I agree that extra care will be required when moving code between libraries. Let me strip this back a bit to only include the "more safe" parts.

This revision was automatically updated to reflect the committed changes.