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

Support baseline configuration #737

Open
wants to merge 11 commits into
base: 8.4.x
Choose a base branch
from

Conversation

lcobucci
Copy link

This allows users to set a list of regexes to filter known BC-breaks from detected changes.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@lcobucci
Copy link
Author

@Nyholm although this uses a different name/format, it could be used to simplify https://github.com/Nyholm/roave-bc-check-docker 👍

@Ocramius
Copy link
Member

I'll need to ponder on this: my preference would be to have either an XSD or a JSON schema, at least, and use those as documentation too 🤔

@Ocramius Ocramius added this to the 8.4.0 milestone Feb 15, 2023
@lcobucci
Copy link
Author

I'll need to ponder on this: my preference would be to have either an XSD or a JSON schema, at least, and use those as documentation too 🤔

I totally understand. I didn't want to introduce new dependencies to the project (I did implicitly add ext-json, though).

Up to you, mate!

src/Baseline.php Outdated Show resolved Hide resolved
src/Baseline.php Show resolved Hide resolved
Copy link
Member

@asgrim asgrim left a comment

Choose a reason for hiding this comment

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

@lcobucci I'm loving this change, and it indeed formalises things that @Nyholm had done before (which I've used for a project too!) - really nice feature, looking forward to getting this shaped up ❤️

src/Baseline.php Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/Command/AssertBackwardsCompatible.php Outdated Show resolved Hide resolved
src/Command/Configuration.php Outdated Show resolved Hide resolved
src/Command/Configuration.php Outdated Show resolved Hide resolved
@lcobucci lcobucci force-pushed the support-baseline-configuration branch from 0680f22 to c251f5a Compare February 23, 2023 22:23
@lcobucci lcobucci force-pushed the support-baseline-configuration branch 2 times, most recently from f208c41 to f9cb116 Compare February 26, 2023 21:08
@lcobucci lcobucci requested review from asgrim and Ocramius and removed request for asgrim and Ocramius February 26, 2023 21:09
@Ocramius
Copy link
Member

@lcobucci sorry, this is on my plate, didn't get to it though :(

@Ocramius Ocramius self-assigned this Feb 26, 2023
@lcobucci
Copy link
Author

@lcobucci sorry, this is on my plate, didn't get to it though :(

Wow, the GH ui went crazy there... Just wanted to ask a review from you two (not to spam both).

No worries, we do what we can, whenever we can!

@lcobucci
Copy link
Author

That segfault seems to be unrelated... let me know if you need help with understanding why it's happening

Copy link
Member

@asgrim asgrim left a comment

Choose a reason for hiding this comment

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

No idea what the segfault is for, but it's happening consistently 😬

README.md Outdated Show resolved Hide resolved
resources/schema.xsd Show resolved Hide resolved
Copy link
Member

@asgrim asgrim left a comment

Choose a reason for hiding this comment

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

Nice one! ❤️ love this change

README.md Outdated Show resolved Hide resolved
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

Looking good!

My only last concern is reducing public API as a start, perhaps, but I also don't mind bumping major version, if the API needs to change due to different directions taken in future 💪

README.md Outdated Show resolved Hide resolved
resources/schema.xsd Show resolved Hide resolved
src/Configuration/Configuration.php Show resolved Hide resolved
@lcobucci lcobucci force-pushed the support-baseline-configuration branch from c39d76d to aeac38f Compare March 1, 2023 21:03
@lcobucci
Copy link
Author

lcobucci commented Mar 1, 2023

All done 👍

@lcobucci lcobucci force-pushed the support-baseline-configuration branch 3 times, most recently from 4dab6f3 to e19974d Compare March 1, 2023 22:49
Some people - like me - have a tweaked/opinionated global Git
configuration that forces GPG signatures everywhere, which basically
breaks the tests.

This standardises the expected configuration for tests, preventing
unexpected breakages.

Signed-off-by: Luís Cobucci <[email protected]>
PHPUnit 10.x only accepts static data providers, so...

Signed-off-by: Luís Cobucci <[email protected]>
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

Pristine job as usual, thanks @lcobucci! 💎

@Ocramius
Copy link
Member

Ocramius commented Mar 2, 2023

Hmm, the segfault part still requires a PHPUnit upgrade: checking that first then.

Given the plans for laminas-continuous-integration-action v2 is to always
ship PCOV, we can simply enable it here to allow us to detach the two
processes.

Signed-off-by: Luís Cobucci <[email protected]>
This adds (lazy) filtering capabilities to detected changes.

Signed-off-by: Luís Cobucci <[email protected]>
This integrates the changes filter into the backward compatibility check
command, parsing the configuration file prior to any process to ease the
integration of new properties.

Signed-off-by: Luís Cobucci <[email protected]>
This moves responsibilities around and introduces abstractions to
support different file formats with less effort.

Signed-off-by: Luís Cobucci <[email protected]>
This ships a parser and the expected schema to ease the validation of
XML documents.

Signed-off-by: Luís Cobucci <[email protected]>
Signed-off-by: Luís Cobucci <[email protected]>
@lcobucci lcobucci force-pushed the support-baseline-configuration branch from e19974d to cdeca27 Compare March 2, 2023 22:50
@@ -26,9 +29,9 @@
"require-dev": {
"doctrine/coding-standard": "^11.0.0",
"php-standard-library/psalm-plugin": "^2.2.1",
"phpunit/phpunit": "^9.5.27",
"phpunit/phpunit": "^10.0.14",
Copy link
Member

Choose a reason for hiding this comment

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

Depends on #741 to be merged

Comment on lines +280 to +288
$output = Shell\execute(__DIR__ . '/../../../bin/roave-backward-compatibility-check', [
'--from=' . $this->versions[0],
'--to=' . $this->versions[1],
], $this->sourcesRepository, [], Shell\ErrorOutputBehavior::Append);

self::assertStringContainsString(
'.roave-backward-compatibility-check.xml" as configuration file',
$output,
);
Copy link
Contributor

Choose a reason for hiding this comment

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

you can use Shell\ErrorOutputBehavior::Packed and [$stdout, $stderr] = Shell\unpack($data) to ensure that the string is in stderr not in stdout.

as of now, if the output goes to stdout instead of stderr, this test will still pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants