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

Document conventions for abstract class naming. #922

Closed
wants to merge 1 commit into from
Closed

Document conventions for abstract class naming. #922

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Dec 18, 2011

No description provided.

@stof
Copy link
Member

stof commented Dec 18, 2011

the point here is that it is not always done. @fabpot thoughts about it ?

@schmittjoh
Copy link
Contributor

I think it's mostly me who doing this, haven't seen anyone else doing it.

@ghost
Copy link
Author

ghost commented Dec 21, 2011

There is no harm adding this now for use from now onwards. The naming conventions really help a lot, even if it's not always done in the Sf2 core. Naming conventions make the code look good, and make it clear to understand what is what.

@yoannch
Copy link

yoannch commented Jun 13, 2012

Excuse me, but is this an applied coding standard in Symfony 2? Or is it still pending and being discussed?

@hppycoder
Copy link

I agree with this convention, having the Abstract as the prefix does give hinting to the implementations.

@stephpy
Copy link
Contributor

stephpy commented Jul 9, 2012

👍

@weaverryan
Copy link
Member

This is not a coding standard that's actually followed in all cases in the core, but it is probably a good idea.

It's a really small deal either way, but this will ultimately probably need the thumbs up/down from @fabpot

Thanks!

@Nemesisprime
Copy link

+1
A little standardization is never frowned upon.

@carlalexander
Copy link

+1

@ghost
Copy link
Author

ghost commented Dec 15, 2012

bump.

@weaverryan
Copy link
Member

Hi guys!

We still don't really have a consensus one way or another on this, but I've patched this into the 2.0 branch at sha: f39b4c4 with a tweak to make the language less strong at sha: a4ca0f8. Hopefully it looks like a nice recommendation, but the language is consistent with the fact that this is not done in core in many cases. And if we hate this, we can always reverse later - but this pushes the issue along :).

Thanks!

@weaverryan weaverryan closed this Dec 16, 2012
@ghost
Copy link
Author

ghost commented Dec 16, 2012

I know there is not consistency, but it's definitely the growing trend, even in Symfony. For new stuff it makes very much good sense to implement (and is what's happening in many places in the PHP world - something we discovered on the PHP-FIG group during the last discussions of PSR-3). It would be better if you put a foot note in the standards to say that in the beginning some of these things were not followed, but we should following them looking forward. You can see the overwhelming support garnered for this: here and a poll (which I cant find atm) found most of the big projects are following this convention looking forward. I would therefor suggest removing recommends, and just adding a footnote to explain why there is disparity in some cases.

weaverryan added a commit that referenced this pull request Dec 16, 2012
…ten a good idea, but definitely not always followed in core
@weaverryan
Copy link
Member

@Drak I also like the suffix, though looking through the core code where it's not used, there are a few places where - if we did use it - it feels awkward. For example, we have the non-static HttpKernel in the component, then it's abstract sub-class HttpKernel in FrameworkBundle. If we did everything all over again, would we even prefix these classes with Abstract?

Let me know your thoughts - I'm happy either way - and I think this is a pretty minor issue - but I want a bit of a consensus/mandate before using stronger language for a standard that we don't see in all of core.

Thanks!

@ghost
Copy link
Author

ghost commented Dec 17, 2012

I've submitted a PR which some suggested wording. It's very important the language is not MAY but MUST because Symfony is very much using Abstract nowadays as are many many large OSS projects. This is very much a convention already. We've delayed this update to the documentation for way too long which only serves to further make the ambiguity.

in direct answer to your question, yes, it definitely would be AbstractHttpKernel if it was written from scratch. Look at the Form component as an example. #2033

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.

8 participants