search

maglnet / ComposerRequireChecker

A CLI tool to check whether a specific composer package uses imported symbols that aren't part of its direct composer dependencies
MIT License
887 stars 71 forks source link

Allow specifying a relative file via phar, assert that the requested file exists #511

Closed fredden closed 7 months ago

fredden commented 7 months ago

There are two main changes in this pull request:

  1. Relative paths now work both via Composer installation and when using the phar file. This fixes #140.
  2. The file provided by the --config-file argument must exist. Previously when the argument was the same as the default, then missing files would be silently ignored. This fixes #507.
fredden commented 7 months ago

I've added a couple of tests to cover the "default argument value" part of this change. The other changes are only visible with the phar. What's the best way to add tests for that?

Ocramius commented 7 months ago

The other changes are only visible with the phar.

That's because the relative path is broken, right? Any way we can convince the suite of a changed CWD? :thinking:

fredden commented 7 months ago

I've opened #515 to get some test coverage of the phar file. When that lands, I'll update this pull request to include a check for the phar bug we're fixing here.

Ocramius commented 7 months ago

@fredden I see this being regularly rebased, but staying red: what are you planning for it, specifically? 🤔

fredden commented 7 months ago

I get notifications when there are merge conflicts, which I then resolve. After #512 has landed, I expect the tests here to pass. As far as I know, the changes here are good, but the test suite is currently incompatible, which is why #512 is open.

(For #512, I am currently spinning up a Windows virtual machine so I can resolve the test failures over there.)

Ocramius commented 7 months ago

But then I see #512 also red? :D

fredden commented 7 months ago

But then I see https://github.com/maglnet/ComposerRequireChecker/pull/512 also red

For now, yes. As mentioned, I'm working on getting a copy of Windows so I can make that green.

Ocramius commented 7 months ago

Thanks, that's the context I was missing! :muscle: