Provide a .phar package for download to avoid requirement conflicts with composer

[hollodotme]

I can see that you already have a box.json present in the repo. Would be great if you could provide a .phar file for download in Github's release section.

[pbrisbin]

Hi @hollodotme that sounds useful. I've added the help-wanted label here because I'm not sure we'll be able to action that work very quickly, so if someone wanted to come along and supply the needed release script (for example) to make that happen, it'd be much appreciated.

[hollodotme]

@pbrisbin OK, I'd like to take care of that and try to provide a PR in the next few days.

[hollodotme]

I had a look into the code today and have some questions:

• Is there a reason why there is no vendor\project namespace? Currently it's just CodeClimate, IMO it should be CodeClimate\PhpTestReporter
• Is there a reason why the TestReporterCommand gets the rootDir? It is not used inside the command. Would make building the PHAR much easier, if it was obsolete.
• Is there a reason why the app has no commands (cleared in extending TestReporterBundle\Console\Application class)? IMO this limits the tool for further features. Anyway for the PHAR I could use the standard Symfony\Console\Application class, so there won't be a BC break.

I'd like to do/add the following:

• Add own stub script for the PHAR
• An upload command with the current options of TestReporterCommand
• A self-update command for self-updating the PHAR (GitHub strategy, needs padraic/phar-updater dependency)
• A rollback command for rolling back the last self-update
• Retain the default commands / options from Symfony console
• Use the Symfony console logger (needs psr/log dependency)
• Add a proper CHANGELOG.md following keepachangelog.com
• Add a short guide for the maintainer how to distribute the built PHAR using Github incl. how to get releases compatible for phar.io.

Feedback appreciated! :)

[pbrisbin]

Is there a reason why there is no vendor\project namespace?

This is most likely due to our own unfamiliarity with PHP practices. This tool's interface is through executing the command, not as a library so I don't think there's any problem in fixing the namespacing to align with community practices.

Is there a reason why the TestReporterCommand gets the rootDir

This seems like some dead code. I can see we set a separate $rootDir variable in the collector itself from getcwd(), so the attribute on the command class appears unused to me.

Is there a reason why the app has no commands

I think it was just that a command-less invocation was a simpler UX for a tool that only does one thing. Requiring the user invoke test-reporter upload instead of just test-reporter seemed unnecessary.

If you added an upload command, does that mean invoking without any command would stop working? You mentioned there "wont be a BC break", so maybe I'm misunderstanding something.

Aside from that question about upload, everything else you've proposed sounds great to me. Were you thinking of breaking up these changes into a few PRs, or all in one go?

[hollodotme]

If you added an upload command, does that mean invoking without any command would stop working? You mentioned there "wont be a BC break", so maybe I'm misunderstanding something.

There won't be a BC break when installing the test-reporter via composer as a dependency, as of now. But I would add the upload and other commands to the PHAR executable (only) right from the start. This is possible by using another stub-script for the PHAR, than the one currently used in the composer.json's "bin" config.

I can break all changes up in several branches and PR's if you like.

One last question before I start: Is there a reason why the project is structured as a Symfony bundle? Is it used as one? If not, I'd like to change the structure first in favour of the vendor/project namespace and use the PSR-4 naming spec.

Thanks.

[pbrisbin]

There won't be a BC break when installing the test-reporter via composer as a dependency, as of now. But I would add the upload and other commands to the PHAR executable (only) right from the start. This is possible by using another stub-script for the PHAR, than the one currently used in the composer.json's "bin" config.

Gotcha. I think that all makes sense.

Is there a reason why the project is structured as a Symfony bundle? Is it used as one? If not, I'd like to change the structure first

Originally this project was modeled after (I think) the Coveralls reporter, so any project structure would have been inherited from that. IMO, so long as the external behavior is unchanged, re-structuring in accordance with a style like PSR-4 sounds awesome.

[hollodotme]

Ok, fine. Gonna start seeding PR's tomorrow. :)

[pbrisbin]

Thanks!

[hollodotme]

All done. :)

You can merge the PRs one by one or simply the last one. I added instructions on how to build and distribute the .phar in the DEVELOPING.md.

Everything should work as usual + now having a PHAR tool to do the job.

Unfortunately Github won't let me upload the built PHAR here. -.-

[pbrisbin]

Thanks so much @hollodotme !

If it's OK, I'd like to do the following (not yet sure when I'll have time, but hopefully this or next week):

• In your first PR, add Code Climate integration to enforce PSR-4 (and other checks)
• Ensure that we're reporting test coverage with ourselves (seems like the best possible integration test for these broad, hopefully behavior-neutral PRs)
• Merge each PR in order, rebasing and reviewing the smaller diffs along the way

[hollodotme]

Of course, take your time. Just let me know if you have any questions or want some additions to the PR branches.

[localheinz]

Closing as we're building a PHAR, just not automatically yet. Working on it!

👍