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 enough metadata to work with PEAR's CI server #7

Closed
wants to merge 8 commits into from

Conversation

CloCkWeRX
Copy link
Contributor

Hey Greg, some changes I've done to get jenkins happy.
http://test.pear.php.net:8080/job/PHP_CodeSniffer/ points at the pear fork at the moment, so I have to pull your changes into there & push before it will run (there must be a better way to track changes, just ignorance on my part I'm guessing).

Based on your email, I thought I'd drop the line length sniff from the ruleset being used to eliminate noise (ignoring all warnings seemed like overkill).

@gsherwood
Copy link
Member

The LineLength cleanup was good and I've merged it in. It should always have been returning as early as possible. Just poor code in there.

The changes for PEAR's CI system are not something I want to included with PHPCS. A fresh clone shouldn't have a whole bunch of files that never get used in other projects or checkouts. If there are required for the PEAR CI itself, then surely they should be in the CI install and not committed to a generic package.

I'm not too concerned if you just drop PHPCS from the PEAR CI anyway. I don't use it (PHPCS is pretty simple) so I don't need you to spend time maintaining it.

@edorian
Copy link
Contributor

edorian commented Jan 23, 2012

Having a build.xml (with the added cli tasks) would make contributing easier as people can do the coding standard checking them self though :)

@gsherwood
Copy link
Member

How does making people use a CI server help lower the barrier of entry for contributions? All they have to do is run phpcs on their code and run the unit tests. You don't need a CI server for that.

Most people don't tend to do it anyway, so I end up writing the unit tests and formatting the code. But that doesn't bother me because I want the contributions. However I get them doesn't matter. It's the ideas that are important.

@CloCkWeRX
Copy link
Contributor Author

@gsherwood happy to leave the build.xml just on the pear fork - it's just in there so that I could point jenkins at your repo and have it build that happily.

@edorian
Copy link
Contributor

edorian commented Jan 24, 2012

@gsherwood I wasn't talking about a CI server. What I mean is that having a file that contains how phpcs and phpunit are supposed to be run (especially with the added phpmd, phpci cli output tasks) helps knowing the patches are like the projects wants them.

Anyways. If you don't mind reformatting and in your experience people wouldn't bother (I do ;) ) then never mind :) Sorry for hijacking this a little :)

@kukulich
Copy link
Contributor

@gsherwood I agree with @edorian. I would be very happy if I could just run ant phpcs or ant phpunit than have to always find out how to run them for PHPCS.

@CloCkWeRX
Copy link
Contributor Author

Uh oh, Jenkins has a posse?

@gsherwood
Copy link
Member

I've added a section in the README for contributing, which shows the 2 commands you would need to run (phpcs and phpunit) to test the code, along with the expected output

I don't use Java tools to test my PHP code, so there is no point including any of the build files given I'd never actually test the code for correctness, so I'd never know if they stopped working. But those 2 commands are the ones I use all the time, so they should be documented, and now are.

Hope that clears up my position on this. I'm closing the pull request. Thanks again for the line length sniff changes.

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.

4 participants