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

Add class configuration functionality instead of php files #122

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

sseidelmann
Copy link

Here is a configuration enhancement for passing classes in configuration files instead of files. It will look like:
version-generator: "Liip\\RMT\\Tests\\Unit\\Config\\ExternalClasses\\CustomVersionPersister"

Now you can use your classes with your own composer autoloading.

@ppetermann
Copy link
Contributor

  • this PR breaks tests
  • the phpunit version added is quite limited, that should be wider
  • the new files don't end on a new line

generally: a pull request should be one feature, so php unit should be one, class config options another, however this isn't too bad with the phpunit stuff as thats just a minor change.

@sseidelmann
Copy link
Author

@ppetermann For PHPUnit see #121

Imho the php version support pre 5.6 should be dropped.
It would make everything a little bit easier

@jeanmonod
Copy link
Member

@sseidelmann As you can see RMT is an old project that deserve some care... Anyway thanks for all your efforts so far! If I get it right we need one more PR, #124, from @acrobat to have a better PHPUnit setup before been able to merge this final PR. Right?

@lsmith77
Copy link
Contributor

I agree to drop at least php 5.3 .. imho 5.5. should only be drop if creates a lot of hackery

@sseidelmann
Copy link
Author

@lsmith77 @jeanmonod When merge the PR #124 I will check the code again and give some updates later.

@dbu
Copy link
Member

dbu commented Nov 23, 2017

#124 is merged

@BradCrumb
Copy link

Any news on this? I really like the change.

@jeanmonod
Copy link
Member

@sseidelmann as #124 is merged, do you want to try to merge master into your branch an see if the tests are still broken?

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.

6 participants