Skip to content

Relocate files to remove redundant directories#288

Merged
acoulton merged 2 commits intoBehat:masterfrom
uuf6429:chore/relocate-files
Feb 22, 2025
Merged

Relocate files to remove redundant directories#288
acoulton merged 2 commits intoBehat:masterfrom
uuf6429:chore/relocate-files

Conversation

@uuf6429
Copy link
Contributor

@uuf6429 uuf6429 commented Feb 19, 2025

This PR does what was suggested here. I thought it would make sense / be cleaner to make a separate PR.

Changes:

  1. ./src is now loaded as PSR-4
  2. flattened files in src/Behat/Gherkin/ to src/
  3. flattened files in tests/Behat/Gherkin/ to tests/
  4. ./src represents Behat\Gherkin\
  5. ./tests represents Tests\Behat\Gherkin\
  6. fixed namespace of existing test

⚠️ I've added a git-ignore-blame entry, so this PR should not be squashed when accepted.

@uuf6429 uuf6429 force-pushed the chore/relocate-files branch from 888f0c6 to 6ef55c7 Compare February 19, 2025 21:37
@acoulton acoulton force-pushed the chore/relocate-files branch from bfd3324 to 9ff6a67 Compare February 19, 2025 23:48
Copy link
Contributor

@acoulton acoulton left a comment

Choose a reason for hiding this comment

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

@uuf6429 I rebased and force pushed this to resolve the conflict with #282 (as that PR still had the previous proposal for changing the composer.json autoload config).

The commits are still showing as yours but now showing a warning that they're unverified (as I obviously can't sign as you). Feel free to rebase and force push again to resolve that.

I have dropped the commit with the git-blame-ignore (the commit ref was outdated anyway) - AFAIK this is not actually required/appropriate for straight file moves, if you look at the blame on https://github.com/uuf6429/Gherkin/blame/chore/relocate-files/tests/ParserTest.php you'll see that it is correctly showing the history despite the file move.

I don't think that conflicts with other PRs will be an issue - the majority are old and have conflicts already anyway, and I've generally found git is usually quite reliable about resolving file renames so it should be quick enough to update anything that needs it.

@uuf6429 uuf6429 force-pushed the chore/relocate-files branch from 9ff6a67 to d7dc340 Compare February 20, 2025 18:48
@uuf6429
Copy link
Contributor Author

uuf6429 commented Feb 20, 2025

@acoulton done, thanks for taking the time to fix things.

Copy link
Contributor

@acoulton acoulton left a comment

Choose a reason for hiding this comment

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

Great, thanks :-)

@acoulton acoulton merged commit 5edc2f5 into Behat:master Feb 22, 2025
8 checks passed
@uuf6429 uuf6429 deleted the chore/relocate-files branch February 23, 2025 00:34
acoulton added a commit to acoulton/Gherkin that referenced this pull request Feb 26, 2025
As expected, this needs to be updated to reflect the file having
moved in Behat#288.
acoulton added a commit to acoulton/Gherkin that referenced this pull request Feb 26, 2025
As expected, this needs to be updated to reflect the file having
moved in Behat#288.
@mvorisek
Copy link
Contributor

mvorisek commented May 8, 2025

Hi, behat/gherkin v4.13.0 release has broken our CI - https://github.com/atk4/ui/actions/runs/14864378786/job/41737354734.

I have verified this is the only changed dependency. If I downgrade to v4.12.0, the issue is gone.

Our Behat configuration is https://github.com/atk4/ui/blob/6.0.0/behat.yml.dist. Quite minimal config.

Is there a bug or do we need to change something?

@acoulton
Copy link
Contributor

acoulton commented May 8, 2025

@mvorisek apologies for the inconvenience. This is tracked at #317 - basically, you need to either update behat or pin to behat/gherkin 4.12.

In your case:

  • I can see from the GitHub Actions logs that your CI is installing behat/behat v3.15.0 (this release is no longer supported)
  • That is because your composer.json requires behat/mink-extension which was officially abandoned 2 years ago (but has not been updated since 2018 and was forked to friends-of-behat/mink-extension in Jan 2020). That is holding you back to symfony/config v4.0, which is unsupported since Jan 2019, and that in turn is holding back several other packages including the more recent Behat releases.

If you switch from behat/mink-extension to friends-of-behat/mink-extension you will get the drop-in replacement with updated dependency constraints, which will resolve this as well as generally unblocking several of your dependencies. I've sent you a PR in atk4/ui#2285

@mvorisek
Copy link
Contributor

mvorisek commented May 8, 2025

Love your perfect guidance, thank you a lot! ❤

@uuf6429
Copy link
Contributor Author

uuf6429 commented May 8, 2025

So to be clear, the issue is not related to this PR (it was only changing the location of test files), right?

@mvorisek
Copy link
Contributor

mvorisek commented May 8, 2025

atk4/ui#2285 fixed the issue for us. This PR is probably fine.

@acoulton
Copy link
Contributor

acoulton commented May 8, 2025

@uuf6429 it is ultimately caused by this - this PR moved the src files as well as the test files, and other packages depended on finding the i18n.php by looking relative to one of the class files.

We discussed it in #290 when it was first merged, and thought we'd found a solution, but we hadn't properly anticipated the impact on non-behat users or that there will often be cases where people can get a more recent gherkin (as it has no deps) but can't get the latest Behat.

@uuf6429
Copy link
Contributor Author

uuf6429 commented May 8, 2025

@acoulton Just checked the PR and your right. I missed that whole discussion and unfortunately the PR description is not really up-to-date. :/ I'll fix that at least.

SidRoberts added a commit to SidRoberts/cron that referenced this pull request Aug 11, 2025
In the subsequent version, files were moved to PSR-4 compliant paths. See Behat/Gherkin#288
SidRoberts added a commit to SidRoberts/cron that referenced this pull request Aug 11, 2025
In the subsequent version, files were moved to PSR-4 compliant paths. See Behat/Gherkin#288
SidRoberts added a commit to SidRoberts/phalcon-cron that referenced this pull request Aug 12, 2025
In the subsequent version, files were moved to PSR-4 compliant paths. See Behat/Gherkin#288
SidRoberts added a commit to SidRoberts/phalcon-cron that referenced this pull request Aug 12, 2025
In the subsequent version, files were moved to PSR-4 compliant paths. See Behat/Gherkin#288
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants