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

fixing examples #4894

Merged
merged 1 commit into from Jan 11, 2016
Merged

fixing examples #4894

merged 1 commit into from Jan 11, 2016

Conversation

spekulatius
Copy link
Contributor

PSR-2 style for examples

PSR-2 style for examples
@spekulatius
Copy link
Contributor Author

Hello @dhensby

do you want to merge this in quickly?

Cheers,
Spek

@tractorcow
Copy link
Contributor

That's not actually psr-2 though :) Or did you just mean tab to space conversion?

@tractorcow
Copy link
Contributor

I'm not sure why this is going into 3.1 branch; Is there a context that I'm missing?

@dhensby
Copy link
Contributor

dhensby commented Jan 6, 2016

Hmm. How does out docs viewer like the space changes?

@tractorcow
Copy link
Contributor

I think it works with 4xspaces the same as it does tabs.

@SpiritLevel
Copy link
Contributor

Happy New Year SilverStripers! I hope everyone is relaxed and refreshed, especially after the stunning holiday weather we have had here in NZ.

@tractorcow, @dhensby...quick question: Is the aim to eventually have all SS code strictly following PSR-2 ? Is there a summary or discussion of which parts of PSR-2 you want SS to strictly adhere to ?
I have noticed that a lot of SS code has 8 spaces but PSR-2 only requries 4, and that PSR-2 requires opening "{" to be on a new line whereas SS typically leaves this on the same line as the class or function declaration. There are other things as well. I am reading a lot of the SS internal code lately and would be happy to convert to PSR-2 as I go. I don't want to irritate you with a flood of PRs so if you let me know what you'd like done, I'd be happy to chip away at it :) Or, are you going to get helpfulrobot onto it ?

@tractorcow
Copy link
Contributor

Is the aim to eventually have all SS code strictly following PSR-2 ?

Yes, but it'll be a gradual adoption rather than immediate. I expect some non-psr-2 names will stick around for the time being.

Framework is currently not psr-2 at all, but we'll do a reformat before releasing 4.0.

Nothing in 3.x will change; It's too much of a maintenance hassle, so any reformatting should be done in master only.

@SpiritLevel
Copy link
Contributor

OK...Master only :)

Is it just me, or do other people cringe at the sight of camel case as well ? Same with putting "{" on new lines. I much prefer K&R style bracing. Oh well, these are minor and there are benefits to conforming...

@tractorcow
Copy link
Contributor

I think the benefit of adopting psr-2 is having to avoid all those conversations. :) Someone somewhere has already argued about all these boring things, and psr-2 is the result.

@SpiritLevel
Copy link
Contributor

I know, I know...I just had to get my two cent's worth in! :P From this point forward I'll be a religious zealot, and follow the most important commandment: Thou shalt not put any other coding styles before PSR-1,2,....

@tractorcow
Copy link
Contributor

The only annoying part is that we'd recently re-named some methods to FOLLOW the silverstripe convention. If we'd left them, they'd be closer to psr-2 than they are now. :)

@SpiritLevel
Copy link
Contributor

That reminds me of Dan's recent comment about fixing a bug and then coming across implementations that require the bug! Double handling is painful not only in coding but also in moving furniture, a currently felt pain since I moved house over the holidays and am still unpacking boxes and shifting furniture around.

@tractorcow
Copy link
Contributor

Yeah, @dhensby, stop putting bugs in other people's changes. :P

dhensby added a commit that referenced this pull request Jan 11, 2016
DOCS moving to spaces for some examples
@dhensby dhensby merged commit 65a6f3d into silverstripe:3.1 Jan 11, 2016
@spekulatius spekulatius deleted the patch-1 branch January 11, 2016 21:50
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.

None yet

4 participants