Page MenuHomeDevCentral

Improve code readability: 8 Mb, restart operation
ClosedPublic

Authored by xcombelle on Aug 6 2016, 19:49.
Referenced Files
Unknown Object (File)
Sun, Nov 24, 08:48
Unknown Object (File)
Sun, Nov 24, 08:48
Unknown Object (File)
Sun, Nov 24, 08:48
Unknown Object (File)
Sun, Nov 24, 08:48
Unknown Object (File)
Sun, Nov 24, 08:25
Unknown Object (File)
Sun, Nov 24, 07:58
Unknown Object (File)
Sun, Nov 24, 07:54
Unknown Object (File)
Sun, Nov 24, 07:40

Details

Summary

This change improves clarity of code:

  • An unit was expressed in bytes, requiring to know 1024 multiples to figure it.
  • Explain the sv restart phd line at the end.
Test Plan
$ sh
$ echo $((8*1024*1024))
8388608  

Diff Detail

Repository
rDPHAB Docker image for Phabricator
Lint
No Lint Coverage
Unit
No Test Coverage
Branch
install
Build Status
Buildable 887
Build 1028: arc lint + arc unit

Event Timeline

xcombelle retitled this revision from to replace magic number of 8Mo and add comment.
xcombelle updated this object.
xcombelle edited the test plan for this revision. (Show Details)
xcombelle added a reviewer: dereckson.

Test plan is what has been used to check.

Personally, I did that:

ysul
$ sh
$ echo $((8*1024*1024))
8388608  
files/usr/local/bin/setup-phabricator
50

If we want to explain final steps what about somehing like this?

# We're done with configuration.
# Final step is to restart the Phabricator daemons to refresh configuration.
# The .initialized file means "this script has been launched and succeeded".

I wonder if the shell variables should not be quoted for example

"$MYSQL_ENV_MYSQL_ROOT_PASSWORD" instead of $MYSQL_ENV_MYSQL_ROOT_PASSWORD

to take account that they could contain shell separator such as space

files/usr/local/bin/setup-phabricator
50

looks good.
maybe the second sentence should be

# Final step is to restart the Phabricator daemons to reload configuration.

(reload instead of refresh)

dereckson retitled this revision from replace magic number of 8Mo and add comment to Improve code readability: 8 Mb, restart operation.Aug 7 2016, 11:57
dereckson updated this object.
dereckson edited the test plan for this revision. (Show Details)
dereckson edited edge metadata.
xcombelle updated this object.
xcombelle edited the test plan for this revision. (Show Details)
  • adding quote to command arguments

The purpose of this commit is to avoid interpretation of special
character by shell

dereckson edited edge metadata.
This revision is now accepted and ready to land.Aug 18 2016, 22:58
dereckson edited edge metadata.

Rebased against master.

This revision was automatically updated to reflect the committed changes.