Skip to content
This repository has been archived by the owner on Jun 7, 2023. It is now read-only.

Commit

Permalink
Rename ShipItBaseConfig to ShipItManifest (#117)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #117

And all the variables names too.

We have too many configs in ShipIt:
1. FBShipItConfig
2. FBSourceBranchConfig
3. FBShipItCLIStaticConfig
4. ShipItBaseConfig

This is endlessly confusing for newcomers. "Manifest" means the same thing but it is visually and semantically distinct. I plan on renaming `FBSourceBranchConfig` as soon as I can think of a good name. And `FBShipItCLIStaticConfig` should just be normal fields on the config.

Reviewed By: jurajh-fb

Differential Revision: D23588313

fbshipit-source-id: 44aa4726bf57ca754d8182930dbb62b212b32f0e
  • Loading branch information
bigfootjon authored and facebook-github-bot committed Sep 9, 2020
1 parent c2e0106 commit f703808
Show file tree
Hide file tree
Showing 31 changed files with 184 additions and 180 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ a Changeset, and return a new, modified one.
## Using FBShipIt

You need to construct:
- a `ShipItBaseConfig` object, defining your default working directory, and the directory names of your source and destination repositories
- a `ShipItManifest` object, defining your default working directory, and the directory names of your source and destination repositories
- a list of phases you want to run
- a pipeline of filters, assuming you are using the `ShipItSyncPhase`

Expand Down
4 changes: 2 additions & 2 deletions demo/DemoSourceInitPhase.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ public function getCLIArguments(): vec<ShipItCLIArgument> {
}

<<__Override>>
public function runImpl(ShipItBaseConfig $config): void {
$local_path = $config->getSourcePath();
public function runImpl(ShipItManifest $manifest): void {
$local_path = $manifest->getSourcePath();

/* HH_FIXME[2049] __PHPStdLib */
/* HH_FIXME[4107] __PHPStdLib */
Expand Down
2 changes: 1 addition & 1 deletion demo/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ In this demo, we only want the `fb-examples` folder, so all other paths are stri
## 4. Base Config

When the source and destination repositories are cloned to the local system, they use
the paths provided by the `ShipItBaseConfig`. For the default working dir,
the paths provided by the `ShipItManifest`. For the default working dir,
`/var/tmp/shipit` is the conventional location. While the source and destination repo
names can be anything they are, by convention, the names of the actual repositories.

Expand Down
6 changes: 3 additions & 3 deletions demo/run_importit.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
DemoGitHubUtils,
DemoSourceRepoInitPhase,
ShipItPhaseRunner,
ShipItBaseConfig,
ShipItManifest,
ShipItChangeset,
ShipItCleanPhase,
ShipItPullPhase,
Expand All @@ -36,7 +36,7 @@ public static function filterChangeset(
}

public static function cliMain(): void {
$config = new ShipItBaseConfig(
$manifest = new ShipItManifest(
/* default working dir = */ '/var/tmp/shipit',
/* source repo name */ 'fbshipit-target',
/* destination repo name */ 'fbshipit',
Expand All @@ -60,7 +60,7 @@ public static function cliMain(): void {
];

try {
(new ShipItPhaseRunner($config, $phases))->run();
(new ShipItPhaseRunner($manifest, $phases))->run();
} catch (ShipItExitException $e) {
exit($e->exitCode);
}
Expand Down
4 changes: 2 additions & 2 deletions demo/run_shipit.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public static function filterChangeset(
}

public static function cliMain(): void {
$config = new ShipItBaseConfig(
$manifest = new ShipItManifest(
/* default working dir = */ '/var/tmp/shipit',
/* source repo name */ 'fbshipit',
/* destination repo name */ 'fbshipit-target',
Expand Down Expand Up @@ -61,7 +61,7 @@ public static function cliMain(): void {
];

try {
(new ShipItPhaseRunner($config, $phases))->run();
(new ShipItPhaseRunner($manifest, $phases))->run();
} catch (ShipItExitException $e) {
exit($e->exitCode);
}
Expand Down
20 changes: 10 additions & 10 deletions fb-examples/lib/config/FBShipItConfig.php-example
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ abstract class FBShipItConfig {
$changeset = $changeset
|> ShipItPathFilters::stripExceptSourceRoots(
$$,
$this->getBaseConfig($branch_config)->getSourceRoots(),
$this->getManifest($branch_config)->getSourceRoots(),
)
|> $this->filterSubmodules($$)
|> (
Expand Down Expand Up @@ -250,12 +250,12 @@ abstract class FBShipItConfig {
return $roots;
}

public function getBaseConfig(
public function getManifest(
FBSourceBranchConfig $branch_config,
): ShipItBaseConfig {
): ShipItManifest {
$static_config = $this->getStaticConfig();
return (
new ShipItBaseConfig(
new ShipItManifest(
'/var/tmp/fbshipit',
/* source_dir = */ $static_config['internalRepo'],
Shapes::idx(
Expand Down Expand Up @@ -443,13 +443,13 @@ abstract class FBShipItConfig {
);
}

final public function getImportBaseConfig(
final public function getImportManifest(
FBSourceBranchConfig $branch_config,
): ShipItBaseConfig {
): ShipItManifest {
$static_config = $this->getStaticConfig();
$shipit_base_config = $this->getBaseConfig($branch_config);
$shipit_manifest = $this->getManifest($branch_config);
return (
new ShipItBaseConfig(
new ShipItManifest(
/* base_dir = */ '/var/tmp/fbimportit',
/* source_dir */ Shapes::idx(
$static_config,
Expand All @@ -461,8 +461,8 @@ abstract class FBShipItConfig {
/* source_roots = */ keyset[],
)
)
->withSourceBranch($shipit_base_config->getDestinationBranch())
->withDestinationBranch($shipit_base_config->getSourceBranch())
->withSourceBranch($shipit_manifest->getDestinationBranch())
->withDestinationBranch($shipit_manifest->getSourceBranch())
->withCommitMarkerPrefix(true);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ final class FBImportItBranchResolutionPhase
?IShipItArgumentParser $argument_parser = null,
): ShipItPhaseRunner {
return new ShipItPhaseRunner(
$config_object->getImportBaseConfig($branch_config),
$config_object->getImportManifest($branch_config),
$config_object->getImportPhases($branch_config),
$argument_parser,
);
Expand Down
6 changes: 3 additions & 3 deletions fb-examples/lib/importit/FBRepoInitPhase.php-example
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@ final class FBRepoInitPhase extends \Facebook\ShipIt\ShipItPhase {
}

<<__Override>>
public function runImpl(\Facebook\ShipIt\ShipItBaseConfig $config): void {
public function runImpl(\Facebook\ShipIt\ShipItManifest $manifest): void {
$local_path = $this->side === \Facebook\ShipIt\ShipItRepoSide::SOURCE
? $config->getSourcePath()
: $config->getDestinationPath();
? $manifest->getSourcePath()
: $manifest->getDestinationPath();

/* HH_IGNORE_ERROR[2049] __PHPStdLib */
/* HH_IGNORE_ERROR[4107] __PHPStdLib */
Expand Down
6 changes: 3 additions & 3 deletions fb-examples/lib/shipit/FBRepoInitPhase.php-example
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,10 @@ final class FBRepoInitPhase extends ShipItPhase {
}

<<__Override>>
public function runImpl(ShipItBaseConfig $config): void {
public function runImpl(ShipItManifest $manifest): void {
$local_path = $this->side === ShipItRepoSide::SOURCE
? $config->getSourcePath()
: $config->getDestinationPath();
? $manifest->getSourcePath()
: $manifest->getDestinationPath();

/* HH_IGNORE_ERROR[2049] __PHPStdLib */
/* HH_IGNORE_ERROR[4107] __PHPStdLib */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class FBShipItBranchResolutionPhase extends ShipItPhase {
?IShipItArgumentParser $argument_parser = null,
): ShipItPhaseRunner {
return new ShipItPhaseRunner(
$config_object->getBaseConfig($branch_config),
$config_object->getManifest($branch_config),
$config_object->getPhases($branch_config),
$argument_parser,
);
Expand Down Expand Up @@ -127,7 +127,7 @@ class FBShipItBranchResolutionPhase extends ShipItPhase {
}

<<__Override>>
public function runImpl(ShipItBaseConfig $_config): void {
public function runImpl(ShipItManifest $_manifest): void {
$config_object = $this->configObject;
$branch_configs = $this->getBranchConfigs();
if ($this->repoMetadataFile !== null) {
Expand Down
2 changes: 1 addition & 1 deletion fb-examples/lib/shipit/FBShipItPreview.php-example
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ order to preview the full patch.';
);
foreach ($branch_configs as $branch_config) {
$source_roots =
$config_object->getBaseConfig($branch_config)->getSourceRoots();
$config_object->getManifest($branch_config)->getSourceRoots();
$changeset_paths = Vec\map(
$changeset->getDiffs(),
($diff) ==> $diff['path'],
Expand Down
8 changes: 4 additions & 4 deletions fb-examples/lib/shipit/FBShipItProjectRunner.php-example
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,15 @@ final class FBShipItProjectRunner extends ShipItPhaseRunner {
private ?string $externalBranch = null,
?IShipItArgumentParser $argumentParser = null,
) {
$config = new ShipItBaseConfig(
$manifest = new ShipItManifest(
'/var/tmp/fbshipit',
'shipit',
'shipit',
keyset[],
);
if ($configObject !== null) {
parent::__construct(
$config,
$manifest,
self::getPhases(
$action,
$configObject,
Expand All @@ -51,7 +51,7 @@ final class FBShipItProjectRunner extends ShipItPhaseRunner {
$argumentParser,
);
} else {
parent::__construct($config, vec[], $argumentParser);
parent::__construct($manifest, vec[], $argumentParser);
}
}

Expand All @@ -68,7 +68,7 @@ final class FBShipItProjectRunner extends ShipItPhaseRunner {
'long_name' => 'verbose',
'description' => 'Give more verbose output',
'write' => $_ ==> {
$this->config = $this->config->withVerboseEnabled();
$this->manifest = $this->manifest->withVerboseEnabled();
ShipItScopedFlock::$verbose = ShipItScopedFlock::DEBUG_EXCLUSIVE;
return true;
},
Expand Down
32 changes: 18 additions & 14 deletions src/importit/phase/ImportItSyncPhase.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

use namespace HH\Lib\Str;
use type Facebook\ShipIt\{
ShipItBaseConfig,
ShipItManifest,
ShipItChangeset,
ShipItDestinationRepo,
ShipItLogger,
Expand Down Expand Up @@ -104,14 +104,18 @@ final public function getCLIArguments(
}

<<__Override>>
final protected function runImpl(ShipItBaseConfig $config): void {
final protected function runImpl(ShipItManifest $manifest): void {
list($changeset, $destination_base_rev) =
$this->getSourceChangsetAndDestinationBaseRevision($config);
$this->applyPatchToDestination($config, $changeset, $destination_base_rev);
$this->getSourceChangsetAndDestinationBaseRevision($manifest);
$this->applyPatchToDestination(
$manifest,
$changeset,
$destination_base_rev,
);
}

private function getSourceChangsetAndDestinationBaseRevision(
ShipItBaseConfig $config,
ShipItManifest $manifest,
): (ShipItChangeset, ?string) {
$pr_number = null;
$expected_head_rev = $this->expectedHeadRev;
Expand All @@ -129,27 +133,27 @@ private function getSourceChangsetAndDestinationBaseRevision(
);
}
$source_repo = new ImportItRepoGIT(
$config->getSourceSharedLock(),
$config->getSourcePath(),
$config->getSourceBranch(),
$manifest->getSourceSharedLock(),
$manifest->getSourcePath(),
$manifest->getSourceBranch(),
);
return $source_repo->getChangesetAndBaseRevisionForPullRequest(
$pr_number,
$expected_head_rev,
$config->getSourceBranch(),
$manifest->getSourceBranch(),
$this->applyToLatest,
);
}

private function applyPatchToDestination(
ShipItBaseConfig $config,
ShipItManifest $manifest,
ShipItChangeset $changeset,
?string $base_rev,
): void {
$destination_repo = ImportItRepo::open(
$config->getDestinationSharedLock(),
$config->getDestinationPath(),
$config->getDestinationBranch(),
$manifest->getDestinationSharedLock(),
$manifest->getDestinationPath(),
$manifest->getDestinationBranch(),
);
if ($base_rev !== null) {
ShipItLogger::out(
Expand All @@ -164,7 +168,7 @@ private function applyPatchToDestination(
ShipItLogger::out(" Filtering...\n");
$filter_fn = $this->filter;
$changeset = $filter_fn($changeset);
if ($config->isVerboseEnabled()) {
if ($manifest->isVerboseEnabled()) {
$changeset->dumpDebugMessages();
}
ShipItLogger::out(" Exporting...\n");
Expand Down
6 changes: 3 additions & 3 deletions src/shipit/ShipItBaseConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

use namespace HH\Lib\Str;

final class ShipItBaseConfig {
final class ShipItManifest {
public function __construct(
private string $baseDirectoryPath,
private string $defaultSourceDirectoryName,
Expand Down Expand Up @@ -201,8 +201,8 @@ private static function useRepositoryLock(): bool {
}

private function modified<Tignored>(
(function(ShipItBaseConfig): Tignored) $mutator,
): ShipItBaseConfig {
(function(ShipItManifest): Tignored) $mutator,
): ShipItManifest {
$ret = clone $this;
$mutator($ret);
return $ret;
Expand Down
Loading

0 comments on commit f703808

Please sign in to comment.