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 CI/build testing #56

Open
5 of 9 tasks
jrfnl opened this issue Feb 18, 2020 · 6 comments
Open
5 of 9 tasks

Add CI/build testing #56

jrfnl opened this issue Feb 18, 2020 · 6 comments

Comments

@jrfnl
Copy link
Contributor

jrfnl commented Feb 18, 2020

I'd like to propose adding basic build testing via Travis.

Things which could be included, would, for instance, be:

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.

@jmarcil
Copy link
Collaborator

jmarcil commented Feb 18, 2020

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.

@jrfnl
Copy link
Contributor Author

jrfnl commented Feb 18, 2020

@jmarcil These are easy enough to add for me.

Would you be interested into becoming a maintainer and be in charge of that part?

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:

  • I can't commit to spending x amount of time on the project. I will contribute when I can.
  • I will not merge my own PRs.

Is that ok for you ?

This would need a decision on what code style to use first.

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 Security standard).

@jrfnl
Copy link
Contributor Author

jrfnl commented Feb 18, 2020

PR #60 addresses action points 1, 3, 4 and 5.

@jrfnl
Copy link
Contributor Author

jrfnl commented Mar 10, 2020

@jmarcil Just checking in: have you had a chance to look at my previous response above #56 (comment) ?

@jmarcil
Copy link
Collaborator

jmarcil commented Mar 16, 2020

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.

@jrfnl
Copy link
Contributor Author

jrfnl commented Mar 16, 2020

So with that in mind, I'd like to defer the code style decision and other nice to have in this current issue.

I understand your choice to defer code-style work and will hold off from spending time on that.

Is there anything else we absolutely need for potential unit tests to be able to run?

PR #70 which I've just pulled would address the Unit test run bullet mentioned above.

Once that is merged, we could already consider adding Code Coverage checking as well.

After that, there's not that much remaining anyway.

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.

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.

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

No branches or pull requests

2 participants