-
Notifications
You must be signed in to change notification settings - Fork 84
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 CI/build testing #56
Comments
Those are really nice things to make this a mature project. However, I can't commit myself on supporting this, and working on the current code to achieve good code quality. Would you be interested into becoming a maintainer and be in charge of that part? My skills are better applied on the security know-how and keeping things consistent within the tool than the actual PHP code itself. I'm sure it is technically poorly written at this point since most of code is around a decade old. |
@jmarcil These are easy enough to add for me.
Well, I do kind of maintain a lot of standards already, but if it helps if I'm a co-maintainer to collaborate on this further, then: why not ? Note: some caveats:
Is that ok for you ?
Have you got a strong preference for a particular kind of standard ? If not, I'd propose to use PHPCSDevCS which is a PSR-12 based standard specifically for PHPCS standards which are not aimed at code style (like the |
PR #60 addresses action points 1, 3, 4 and 5. |
@jmarcil Just checking in: have you had a chance to look at my previous response above #56 (comment) ? |
Yes, but forgot to answer your questions. It's OK, as long as I'm the sole person that can merge, I still consider myself alone in the endeavor of maintaining this tool. So because of that, I will have to considerably diminish the amount of work being put on code quality and other refactoring of this tool. I've already spent more time this year on this than actual road-map items people requested since a long. So with that in mind, I'd like to defer the code style decision and other nice to have in this current issue. Is there anything else we absolutely need for potential unit tests to be able to run? If not I think we should close this issue so I can refocus back on things that matter more for the usage of the tool versus the development of it. Thank you for your understanding. |
I understand your choice to defer code-style work and will hold off from spending time on that.
PR #70 which I've just pulled would address the Once that is merged, we could already consider adding Code Coverage checking as well. After that, there's not that much remaining anyway.
I'd prefer to keep the ticket open though until the points have all been addressed, even though they will be low priority and not get immediate focus for now. |
I'd like to propose adding basic build testing via Travis.
Things which could be included, would, for instance, be:
composer.json
file. - PR Add initial CI check #60xsd
schema. - PR Add initial CI check #60This would need a decision on what code style to use first.
In the future, once sniff specific tests have been added this could be expanded to also include:
Please let me know if you're interested in this and if so, if there are any other things you'd like to include or check you'd want to exclude.
The text was updated successfully, but these errors were encountered: