c3 icon indicating copy to clipboard operation
c3 copied to clipboard

Move C3 code to separate class (2023)

Open marcovtwout opened this issue 1 year ago • 6 comments

This PR is a first step in improving https://github.com/Codeception/c3/issues/40 applying the same solution as https://github.com/Codeception/c3/pull/72 Instead of having a copy of c3.php in the project root it can be included from any location.

Basic usage:


require __DIR__ . '/vendor/autoload.php'; // If using Composer
//require __DIR__ . '/vendor/codeception/c3/c3.php'; // Or include it directly

$c3 = new Codeception\C3(__DIR__); // pass the directory that contains Codeception config files
$c3->run();

I only made the minimal changes to use this concept. Best to disable whitespace comparison when reviewing this PR.

There's more to be improved after this:

  • Since this probably means a new major version of c3, we could remove legacy options for outdated autoloading, codeception and phpunit versions
  • Add options to configure error log file and temp output directory
  • Make all __c3_-functions part of the class
  • Restructure remaining code blocks into functions

But please review this PR first and decide if this could start a new v3 branch. If so, I'd be happy to submit further improvement PR's on top of that (based on https://github.com/Codeception/c3/compare/main...marcovtwout:c3:class_based_c3_improvements).

marcovtwout avatar Apr 04 '23 12:04 marcovtwout

@Naktibalda Do you need any help getting this reviewed? I think it would be really benificial for the maintainability of this project if it becomes a composer requirement through this PR.

marcovtwout avatar May 03 '23 14:05 marcovtwout

@TavoNiievez Is there anyone else that can review this?

marcovtwout avatar Aug 21 '23 07:08 marcovtwout

@marcovtwout looks good (haven't tried it), you should resolve conflicts and update this line for the tests here with the latest PHP versions, and drop the ones that are no longer supported https://github.com/Codeception/c3/blob/main/.github/workflows/main.yml#L11 Official support for PHP 8.0 ends in January https://www.php.net/supported-versions.php

delboy1978uk avatar Oct 27 '23 09:10 delboy1978uk

@delboy1978uk That line is not part of this PR, but I can rebase and fix conflicts again and again.. but there is no point until somebody reviews this first.

So can somebody please review this? :)

marcovtwout avatar Dec 18 '23 16:12 marcovtwout

@SamMousa Thanks for your review!

I agree exactly with your conclusions, but I have split them up into two parts for the sake of easier reviewing and continued development:

  • Step 1, create a new major version based on this PR so we can use it as a Composer dependency. The result could already be used by anyone with existing setups, with only a minimal change in your projects and low risk of breaking things.
  • Step 2, start development on the next major version to get rid of many legacy options and improve code quality. For a great part my suggestions overlap with yours. See the commits I created on this derived branch: https://github.com/Codeception/c3/compare/main...marcovtwout:c3:class_based_c3_improvements

marcovtwout avatar Dec 21 '23 11:12 marcovtwout

Cool, go ahead with step 1 then. For the new implementation I'd say have a look at #93! If you want to give hte implementation a try go ahead.

SamMousa avatar Dec 21 '23 12:12 SamMousa