Skip to content

Update infrastructure #161

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

Merged
merged 7 commits into from
Jul 24, 2015
Merged

Update infrastructure #161

merged 7 commits into from
Jul 24, 2015

Conversation

dantleech
Copy link
Member

This PR:

  • Switches to using autoloading
  • Introduce a phpcsfixer configuration (and fix the CS)
  • Removes the numerical prefix from the test directory names (s/[0-9]{2}_//g)

@dantleech
Copy link
Member Author

Have executed the Jackalope-Doctrine-Dbal test suite and it works.

$header = <<<EOF
This file is part of the PHPCR API Tests package

Copyright (c) 2013 Liip and others
Copy link
Member Author

Choose a reason for hiding this comment

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

2015

@dantleech
Copy link
Member Author

Reverted from lib backto inc to preserve BC.

@lsmith77
Copy link
Member

looks good to me!

@lsmith77
Copy link
Member

I guess since we use explicit tagged versions in Jackalope, this should also not cause breakage in older branches .. right?

@dantleech
Copy link
Member Author

Yeah, I guess if we have a policy of using explicit versions for phpcr-api-tests in jackalope, it would be safe. But should not break anything even in older versions atm, as its fully BC (tm).

file that was distributed with this source code.
EOF;

Symfony\CS\Fixer\Contrib\HeaderCommentFixer::setHeader($header);
Copy link
Member

Choose a reason for hiding this comment

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

when / how is this triggered?

Copy link
Member Author

Choose a reason for hiding this comment

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

It would need to be executed manually: /path/to/phpcsfixer fix

Copy link
Member

Choose a reason for hiding this comment

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

we should have that in a doc comment then.

@dbu
Copy link
Member

dbu commented Jul 4, 2015

cool, makes sense. i would love to keep the chapter numbers in the namespaces however. can a php namespace start with a number? then we could namespace them as 06Query or something.

@dantleech
Copy link
Member Author

No, it seems namespaces cannot start with numbers. I would suggest to add the chapter information to the class documentation, and also the relevant parts of the spec to each test method doc (if this is not already the case).

@lsmith77
Copy link
Member

sounds good to me ..

@dbu
Copy link
Member

dbu commented Jul 14, 2015

well, it was very useful to find tests when following the specification. but i guess i will just have to change my habits. the benefits of this PR are worth more than this convenience :-)

the spec is not copied into the tests. sometimes the spec subsections covered are mentioned, but not consistently. while that certainly would be great, it sounds like a huge work. lets document the cs fix thingy a bit and then merge this.

dbu added a commit that referenced this pull request Jul 24, 2015
@dbu dbu merged commit ad3234c into master Jul 24, 2015
@dbu dbu deleted the clean branch July 24, 2015 15:22
@lsmith77 lsmith77 removed the review label Jul 24, 2015
@lsmith77 lsmith77 removed the review label Jul 24, 2015
@dbu
Copy link
Member

dbu commented Jul 24, 2015

@dantleech thanks! now we need to adjust the jackalopes...

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.

3 participants