Comments (18)
That would be nice! Right now I rely on my IDE and on Scrutinizer to check those things.
Basically, I use PSR code style, Symfony standard for service naming, and I prefer spacing present around the !
and .
operators.
from paraunit.
@Jean85 yep, i've looked into the .scrutinizer file and i think we have a couple of option to help people get along with the project current code-style. I think t
The first would be creating a custom ruleset.xml taking all we can from PSR2 that does not trigger a huge scale autofix from PHPCBF. Link that file to both the .scrutinizer and to a custom bash script (contributing.sh) something like:
phpcs -p --standard=/path/to/custom_ruleset.xml --colors src/
phpcbf --standard=/path/to/custom_ruleset.xml --colors src/
The other option will be to enforce the current .scrutinizer configured code-style by creating a custom ruleset with only a couples of rules (spacing present around the ! and . operators.).
I think we should follow the first approach, it may require a bit of work at the beginning but then we can have 3 tools working with a single .xml file.
I've made some digging with the standard PSR2 ruleset of phpcs ruleset.xml i think we can make it work (aka without triggering an all-around autofix) with only removing 3/4 rules.
from paraunit.
Good!
What rules we need to remove? Are we violating PSR-2 in some way? I would prefer not to!
I would likely ignore just the line-length warnings, even the PSR says that is a soft-limit.
[EDIT] Found, the spaces around the !
operator at the start of a control structure violates that. I think we can ignore that, for the sake of readability.
Do you think we can use a pre-commit hook to enforce this formatting?
Also, I agree on the XML proposal. Can you work on it? Maybe in a branch here, without forking, so I can help.
from paraunit.
If you are wondering about a CONTRIBUTING.md
file I'd suggest you to look also at ISSUES/PR templates. Take a look at what docker/docker did there: https://github.com/docker/docker/tree/master/.github
from paraunit.
I see @fntlnz , but we don't have enough traffic to warrant a template like those, at least not for issues... What are you suggesting we should add?
from paraunit.
I was just pointing out that some projects integrates the CONTRIBUTING.md
file with these templates.
On Monday, 21 March 2016, Alessandro Lai [email protected] wrote:
I see @fntlnz https://github.com/fntlnz , but we don't have enough
traffic to warrant a template like those, at least not for issues... What
are you suggesting we should add?—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#41 (comment)
from paraunit.
@Jean85 sure, i'll create a branch so we can all look into.
It's still in heavy wip but if you look at these lines you can see i've commented out some rules. That part triggered a big autofix. I've also inceased the max line lenght to 180 as we violated the 120 in some cases, but as you pointed out it's really a pointless rule to enforce (here).
If you run the contributing.sh script file it should point out any violation so we can work iterativly on adding/editing the ruleset file.
from paraunit.
I've launched the script, and the main result is, apart from some minor fixes that I like, a bunch of !
operators that are no more spaced.
PSR-2 and Symfony follow the no-space convention, I'm unsure about how to proceed. Should we align Paraunit to this standard? Or should we stay with the spaces?
I'm not sure on how to enforce the spacing, in the latter case..
from paraunit.
I'm more inclined to remove the spacing altogether and bring the project closer to the PSR-2 standard.
I think it should be the goal of this branch to set a definitive code style guide so, even if there will be some changes (like the one above) it could make sense to proceed.
from paraunit.
Agreed. Anyhow, using the PSR2 standard, PHPCS leaves alone the space AFTER the !
operator. I think we could leave it, to increase readability without going off-standard.
This should generate a bit of corrections, but it's better do it now and stick with some fixed and well shared rules as PSR are.
from paraunit.
Ok, I've pushed the proposed fixes. Next and last step should be a post-commit git hook to impose the standard.
from paraunit.
Hook is ready, it will be created at the first "composer install" command.
We still need a CONTRIBUTING.md then i think we're good.
from paraunit.
Shouldn't we suppress the output to avoid spamming the console when committing?
[EDIT] Also, shouldn't we impose the standard in the tests folder too?
from paraunit.
@Jean85 i've changed the logic a bit, rather than forcing an autofix now it will only run phpcs and leave the code review job to the developer.
Also if phpcs fails (exit code != 0) the commit will be blocked and the user will see the the reason in the output. It's an hard limit i'm not sure if it's the right way.
Run composer install again to get the updated version of the hook and let me know what you think.
NOTE: i've lowered the severity on warning so we ignore the line limit
[EDIT] Also, shouldn't we impose the standard in the tests folder too?
At the current stage we have a few errors for tests not being camelCase in their method name. We could change that and have it working for the test directory as well.
from paraunit.
The Git hooks can always be avoided with git commit --no-verify
, so it's not an hard limit ;)
I've seen your changes, LGTM.
You would add a --force
or --autofix
option to the script BTW, to force the changes.
from paraunit.
Ok I took a stab at the .md file: https://github.com/facile-it/paraunit/blob/2203_contributing/CONTRIBUTING.md
Please review it!
from paraunit.
LGTM, i've mentioned docker at the beginning with a link to the installation process.
from paraunit.
Good! 👍
Ok, I've fixed and included the tests in the check. I'll open a PR for this branch to discuss and review it further
from paraunit.
Related Issues (20)
- Change injection point
- Differentiate between test outcome and issue
- Use autowiring
- Remove AbstractEvent
- Simplify Process objects
- Mark tests with coverage missing
- Handle `--column` option
- Handle all `--display-*` options HOT 1
- Handle all `--stop-on-*` options HOT 1
- Handle `--exclude-testsuite` and `--test-suffix` options
- Handle `--order-by` and `--random-order-seed` options
- Handle `--warm-coverage-cache` option
- Handle `--log-junit` option HOT 5
- Implement `--print-failed-early` option for version 1.x
- PHPUnit version visibility
- PHPUnit's string comparison diff is not handled by ParaUnit HOT 2
- PHPUnitError event is not handled
- crashing on --exclude-group HOT 1
- Deprecation warnings in >2.0 HOT 1
- Support for PHPUnit 11 HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from paraunit.