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

[WIP] - Inform when there is no minor version #668

Draft
wants to merge 3 commits into
base: 8.0.x
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,19 +58,19 @@ as part of your CI pipeline. For example in a GitHub action, note the use of
jobs:
roave-backwards-compatibility-check:
name: Roave Backwards Compatibility Check
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
with:
fetch-depth: 0
- name: "Install PHP"
uses: shivammathur/setup-php@v2
with:
php-version: "8.0"
- name: "Install dependencies"
run: "composer install"
- name: "Check for BC breaks"
run: "vendor/bin/roave-backward-compatibility-check"
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
with:
fetch-depth: 0
- name: "Install PHP"
uses: shivammathur/setup-php@v2
with:
php-version: "8.0"
- name: "Install dependencies"
run: "composer install"
- name: "Check for BC breaks"
run: "vendor/bin/roave-backward-compatibility-check"
```

#### Nyholm Github Action
Expand Down
52 changes: 22 additions & 30 deletions src/Command/AssertBackwardsCompatible.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@
use Roave\BackwardCompatibility\Git\ParseRevision;
use Roave\BackwardCompatibility\Git\PerformCheckoutOfRevision;
use Roave\BackwardCompatibility\Git\PickVersionFromVersionCollection;
use Roave\BackwardCompatibility\Git\Revision;
use Roave\BackwardCompatibility\LocateDependencies\LocateDependencies;
use Roave\BackwardCompatibility\VersionConstraint\StableVersionConstraint;
use Roave\BetterReflection\SourceLocator\Type\AggregateSourceLocator;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Exception\InvalidArgumentException;
Expand Down Expand Up @@ -115,9 +115,27 @@ public function execute(InputInterface $input, OutputInterface $output): int
// @todo fix flaky assumption about the path of the source repo...
$sourceRepo = CheckedOutRepository::fromPath(Env\current_dir());

$fromRevision = $input->getOption('from') !== null
? $this->parseRevisionFromInput($input, $sourceRepo)
: $this->determineFromRevisionFromRepository($sourceRepo, $stdErr);
if ($input->getOption('from') !== null) {
$maybeRevision = Type\string()->coerce($input->getOption('from'));
} else {
$versions = $this->getVersions->fromRepository($sourceRepo);

Psl\invariant(Iter\count($versions) >= 1, 'Could not detect any released versions for the given repository');

$stableVersions = $versions->matching(new StableVersionConstraint());

if ($stableVersions->isEmpty()) {
$stdErr->writeln(Str\format('Your library does not have any stable versions. Therefore cannot have BC breaks.'));

return Command::SUCCESS;
Comment on lines +127 to +130
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realized another problem, sorry @_@

Assume somebody incorrectly installs this tool with a CI setup that did not pull tags during clone operations (shallow clone: default on github), this leads to an exit code zero by default, hiding the underlying problem.

Perhaps an exception or non-zero exit code was really the best approach.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, maybe we can kill two birds with one stone. One is a misconfiguration which we could verify and the second is an exception :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One is a misconfiguration which we could verify

How do we verify it, if no git fetch has occurred? 🤔

Copy link
Author

@Ferror Ferror Nov 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't that...

No tags -> possible misconfiguration so we can throw an exception. Like. THERE ARE NO TAGS. WE NEED THEM (yes, we gonna scream 😄)

Then at some point, we can assume that there are at least some tags (we verified that by the previous action), but we want to check out the stable ones. There are tags, but none of them are stable. Return success.

What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

THERE ARE NO TAGS. WE NEED THEM (yes, we gonna scream smile)

Works for me - indeed the tool cannot do auto---from= without tags.

Then at some point, we can assume that there are at least some tags (we verified that by the previous action), but we want to check out the stable ones. There are tags, but none of them are stable. Return success.

Good enough :-)

}

$maybeRevision = $this->pickFromVersion->forVersions($versions)->toString();

$stdErr->writeln(Str\format('Detected last minor version: %s', $maybeRevision));
}

$fromRevision = $this->parseRevision->fromStringForRepository($maybeRevision, $sourceRepo);

$to = Type\string()->coerce($input->getOption('to'));

Expand Down Expand Up @@ -187,30 +205,4 @@ private function printOutcomeAndExit(Changes $changes, OutputInterface $stdErr):

return $hasBcBreaks ? 3 : 0;
}

/** @throws InvalidArgumentException */
private function parseRevisionFromInput(InputInterface $input, CheckedOutRepository $repository): Revision
{
$from = Type\string()->coerce($input->getOption('from'));

return $this->parseRevision->fromStringForRepository($from, $repository);
}

private function determineFromRevisionFromRepository(
CheckedOutRepository $repository,
OutputInterface $output,
): Revision {
$versions = $this->getVersions->fromRepository($repository);

Psl\invariant(Iter\count($versions) >= 1, 'Could not detect any released versions for the given repository');

$versionString = $this->pickFromVersion->forVersions($versions)->toString();

$output->writeln(Str\format('Detected last minor version: %s', $versionString));

return $this->parseRevision->fromStringForRepository(
$versionString,
$repository,
);
}
}
9 changes: 2 additions & 7 deletions src/Git/PickLastMinorVersionFromCollection.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@

use Psl;
use Psl\Type;
use Roave\BackwardCompatibility\VersionConstraint\StableVersionConstraint;
use Version\Comparison\Constraint\CompositeConstraint;
use Version\Comparison\Constraint\Constraint;
use Version\Comparison\Constraint\OperationConstraint;
use Version\Version;
use Version\VersionCollection;
Expand All @@ -18,12 +18,7 @@ public function forVersions(VersionCollection $versionsCollection): Version
{
Psl\invariant(! $versionsCollection->isEmpty(), 'Cannot determine latest minor version from an empty collection');

$stableVersions = $versionsCollection->matching(new class implements Constraint {
public function assert(Version $version): bool
{
return ! $version->isPreRelease();
}
});
$stableVersions = $versionsCollection->matching(new StableVersionConstraint());

$versionsSortedDescending = $stableVersions->sortedDescending();

Expand Down
16 changes: 16 additions & 0 deletions src/VersionConstraint/StableVersionConstraint.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

declare(strict_types=1);

namespace Roave\BackwardCompatibility\VersionConstraint;

use Version\Comparison\Constraint\Constraint;
use Version\Version;

final class StableVersionConstraint implements Constraint
{
public function assert(Version $version): bool
{
return ! $version->isPreRelease();
}
}
24 changes: 24 additions & 0 deletions test/unit/VersionConstraint/StableVersionConstraintTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php

declare(strict_types=1);

namespace RoaveTest\BackwardCompatibility\VersionConstraint;

use PHPUnit\Framework\TestCase;
use Roave\BackwardCompatibility\VersionConstraint\StableVersionConstraint;
use Version\Version;

final class StableVersionConstraintTest extends TestCase
{
public function testStableVersions(): void
{
$constraint = new StableVersionConstraint();

self::assertTrue($constraint->assert(Version::fromString('1.0.0')));
self::assertTrue($constraint->assert(Version::fromString('0.1.0')));
self::assertTrue($constraint->assert(Version::fromString('0.0.1')));
self::assertFalse($constraint->assert(Version::fromString('1.0.0-alpha.1')));
self::assertFalse($constraint->assert(Version::fromString('1.0.0-beta.1')));
self::assertFalse($constraint->assert(Version::fromString('1.0.0-rc.1')));
}
}