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

consistent spelling #4671

Merged
merged 16 commits into from
May 22, 2015
Merged

consistent spelling #4671

merged 16 commits into from
May 22, 2015

Conversation

xabbuh
Copy link
Member

@xabbuh xabbuh commented Dec 19, 2014

Q A
Doc fix? no
New docs? no
Applies to all
Fixed tickets #3237
  • stylesheet - Stylesheet
  • front-end - frontend - front end
  • Full-Stack - Full stack
  • Symfony2 Framework - form framework - Form framework
  • Save-handler - Save handler
  • Dependeny Injection Tags - Dependeny Injection Container - dependeny injection container
  • Event Dispatcher - event dispatcher
  • Event Subscriber - event subscriber
  • Symfony2 Standard Distribution - Symfony2 Standard Edition
  • Console Command - console command
  • email - Email - e-mail
  • Boolean - boolean
  • web debug toolbar - Web Debug Toolbar
  • Exception - exception
  • object-oriented - object oriented

@@ -13,7 +13,7 @@ Component. Configuration values are usually expected to show some kind of
hierarchy. Also, values should be of a certain type, be restricted in number
or be one of a given set of values. For example, the following configuration
(in YAML) shows a clear hierarchy and some validation rules that should be
applied to it (like: "the value for ``auto_connect`` must be a boolean value"):
applied to it (like: "the value for ``auto_connect`` must be a Boolean value"):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should the adjective have a capital letter too ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least, that's how Wikipedia uses it (I also think that this make sense since it's based on George Boole's name).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be reverted to lowercase, just like the rest of the occurences

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, there are more Boolean occurences. Looks like you didn't replace them to be lowercase

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still goes wrong here, @xabbuh (seems like all "boolean" occurences are replaced with "Boolean" and all "Boolean" with "boolean")

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed - the PR has a mix of changes right now. irrc, we've started using the lowercase version in core, but I'm not sure.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

core has switched to bool to support Hack. I'm still voting for "boolean"

@wouterj
Copy link
Member

wouterj commented Dec 22, 2014

Do we really want Boolean? I really don't like it as it's inconsistent.

@xabbuh
Copy link
Member Author

xabbuh commented Dec 22, 2014

@wouterj I think it make sense in terms that it is derived from George Boole's name. We also used Boolean in quite a lot of places in the reference for option types. Though I don't have any strong feelings about this. If you all think that boolean feels more natural, I'll revert the changes and convert uppercased occurences with lowercased ones.

By the way, why do we use Boolean in the reference at all and don't use bool like the Symfoy code base does? Is it just for historical reasons?

@javiereguiluz
Copy link
Member

@xabbuh I agree with @wouterj: even if Boolean is the correct term, most developers use it as boolean and treat it as a common word.

@xabbuh xabbuh force-pushed the issue-3237 branch 2 times, most recently from d906731 to c09ba41 Compare December 22, 2014 13:56
@xabbuh
Copy link
Member Author

xabbuh commented Dec 22, 2014

Seems we have a majority for boolean instead of Boolean then. :) Have replaced them accordingly.

@wouterj
Copy link
Member

wouterj commented Dec 22, 2014

@xabbuh the code changed this year to bool instead of Boolean (that's where the capitialized boolean in the docs come from), to make their types compatible with Hack (http://docs.hhvm.com/manual/en/hack.annotations.types.php).

I don't think bool is as clear as boolean, that's why I propose to keep using boolean instead of bool

@xabbuh
Copy link
Member Author

xabbuh commented Dec 22, 2014

@wouterj Thanks for the explanation. So, should we now keep lowercased boolean everywhere or switch it back to Boolean in the reference and use boolean in texts only?

@wouterj
Copy link
Member

wouterj commented Dec 22, 2014

I think we should use boolean (lowercased) everywhere.

@xabbuh xabbuh changed the title [WIP] consistent spelling consistent spelling Dec 29, 2014
@xabbuh
Copy link
Member Author

xabbuh commented Dec 29, 2014

This is ready to be reviewed now. I've kept different commits for each item of the task list so that you can review each step on its own.

However, there are two changes I'm not completely happy with:

  • Unifying occurrences of Symfony framework and Symfony Framework to always use the latter on is probably a good idea. But we also have a lot of occurrences of "Symfony full-stack Framework" now which looks a bit weird. Should we keep it anyway? Or replace it with something like "full-stack Symfony Framework" or "Symfony Full-Stack Framework"?
  • I don't like using uppercase letters for "Dependency Injection". Why don't we write "dependency injection container", "dependency injection tags" and "dependency injection extension class" while keeping uppercase letters only for the "DependencyInjection component"?

@javiereguiluz
Copy link
Member

@xabbuh regarding your questions:

  1. I prefer using this variant: full-stack Symfony Framework
  2. I agree with you about lowercasing dependency injection except when referring to the component.

Do you think this PR could be merged anytime soon? It looks like we're very close thanks to your great work and it's sad to see these nice improvements pending to be merged. Thanks.

@xabbuh
Copy link
Member Author

xabbuh commented Feb 19, 2015

@javiereguiluz This can be merged once we agree on all the changes. :) Though I'm fine with splitting it in several PRs if there are some things that should be discussed while others may be settled.

@wouterj @weaverryan What do you think?

@wouterj
Copy link
Member

wouterj commented Feb 21, 2015

+1 for Symfony Full-Stack Framework
+1 for lowercase dependency injection and only capitialized when talking about the component

Btw, thsi also needs a rebase.

@@ -1519,7 +1519,7 @@ file, you can see every block needed to render a form and every default field
type.

In PHP, the fragments are individual template files. By default they are located in
the `Resources/views/Form` directory of the framework bundle (`view on GitHub`_).
the `Resources/views/Form` directory of the FrameworkBundle (`view on GitHub`_).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be double backticks

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we do this anywhere else with bundle names?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, I meant the resoruces/views/form at the start of the line

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, of course. Totally missed that. Thanks for noticing. 👍

@wouterj
Copy link
Member

wouterj commented Apr 30, 2015

This PR still contains the boolean mix up.

@xabbuh xabbuh force-pushed the issue-3237 branch 2 times, most recently from 24eba9c to f0e0663 Compare May 1, 2015 08:27
@xabbuh
Copy link
Member Author

xabbuh commented May 1, 2015

I rebased and finally fixed the boolean/Boolean mix up.

@wouterj
Copy link
Member

wouterj commented May 21, 2015

Let's merge this one to avoid more conflicts :)

@weaverryan
Copy link
Member

WOW @xabbuh. And not even any conflicts :). Thanks for the long work on this!

@weaverryan weaverryan merged commit f182121 into symfony:2.3 May 22, 2015
weaverryan added a commit that referenced this pull request May 22, 2015
This PR was merged into the 2.3 branch.

Discussion
----------

consistent spelling

| Q             | A
| ------------- | ---
| Doc fix?      | no
| New docs?     | no
| Applies to    | all
| Fixed tickets | #3237

- [x] stylesheet - Stylesheet
- [x] front-end - frontend - front end
- [x] Full-Stack - Full stack
- [x] Symfony2 Framework - form framework - Form framework
- [x] Save-handler - Save handler
- [x] Dependeny Injection Tags - Dependeny Injection Container - dependeny injection container
- [x] Event Dispatcher - event dispatcher
- [x] Event Subscriber - event subscriber
- [x] Symfony2 Standard Distribution - Symfony2 Standard Edition
- [x] Console Command - console command
- [x] email - Email - e-mail
- [x] Boolean - boolean
- [x] web debug toolbar - Web Debug Toolbar
- [x] Exception - exception
- [x] object-oriented - object oriented

Commits
-------

f182121 uppercase "dependency injection"
6b7d536 use "Symfony Framework" instead of "Symfony framework"
07bc4db use "Form component" instead of "form framework"
bd5ca26 use "full-stack" instead of "full stack"
5b6895a unify exception usages
b774651 unify event subscriber usages
95c842c use "console command" instead of "Console Command"
9c02eda unify EventDispatcher/event dispatcher usages
fd52cd3 use boolean instead of Boolean
f8db4b0 use "object-oriented" instead of "object oriented"
1bd33ca "web debug toolbar" instead of "Web Debug Toolbar"
b2d802e use "stylesheet" instead of "Stylesheet"
7eb1847 unify "Symfony Standard Edition" usages
85fb0b1 consistency, replace "save-handler" with "save handler"
83238aa consisteny, use "front-end" instead of "frontend" or "front end"
5b8e84d consistency, replace "e-mail" with "email"
@xabbuh xabbuh deleted the issue-3237 branch May 22, 2015 05:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants