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

Added project dependencies in composer #61

Merged
merged 1 commit into from
Jan 21, 2015
Merged

Added project dependencies in composer #61

merged 1 commit into from
Jan 21, 2015

Conversation

mmoreram
Copy link
Contributor

  • Symfony dependencies (Components and Bundles)
  • FrameworkExtraBundle dependencies

@mmoreram mmoreram mentioned this pull request Jan 20, 2015
"doctrine/orm" : "~2.3",
"pagerfanta/pagerfanta" : "~1.0"
"php" : ">=5.3.0",
"doctrine/orm" : "~2.3",
Copy link
Collaborator

Choose a reason for hiding this comment

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

doctrine/common and doctrine/doctrine-bundle is needed too.

By the way, I would not align the version numbers. It makes the diff mainly useless when only whitespaces change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can't find where these packages are required (namespaces? services?)... could you please show us? :)

And the reason of alignment is because preferences of @javiereguiluz, right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@mmoreram Never mind, seems that I have to sync my local fork. ;)

Copy link
Collaborator

Choose a reason for hiding this comment

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

@xabbuh about the alignment, I know that 99% of developers don't agree with me, but I love to align (and sort alphabetically) the dependencies of the projects.

@mmoreram sorry for bothering you, but could you please sort them alphabetically and add one additional white space before all semicolons (:) to turn this:

"sensio/framework-extra-bundle": "~2.3|~3.0",

into this:

"sensio/framework-extra-bundle" : "~2.3|~3.0",

Thanks a lot!

Copy link
Collaborator

Choose a reason for hiding this comment

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

@xabbuh about the alignment, I know that 99% of developers don't agree with me, but I love to align (and sort alphabetically) the dependencies of the projects.

I forgot about your preferences when commenting. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@javiereguiluz Seriously? ¬¬

Copy link
Contributor

Choose a reason for hiding this comment

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

Hehe, sounds a bit psycho but I love doing the same even if I don't do it all the time.
What's again better is to sort the bundles in AppKernel, by FQCN length AND alphabetically.

_Psycho inside_

😉

What's kinda bad is that whenever you make a composer require {package} in the project root directory, the new composer file won't align the packages names and versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, and I am used to sorting the use statements, but not by hand. If you want to ensure that everything is aligned with your style, make sure you provide a mechanism for such, like php-formatter.

Copy link
Contributor

Choose a reason for hiding this comment

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

IDEs code formatters can work, especially PhpStorm's one which is very good at it, and fully parametrable

@javiereguiluz
Copy link
Collaborator

@mmoreram thanks for working on this. Should we also add twig as a dependency? We use it for templates and we even include a Twig extension.

@mmoreram
Copy link
Contributor Author

@javiereguiluz Absolutely. I'll update the branch before lunch! 👍

* Symfony dependencies (Components and Bundles)
* FrameworkExtraBundle dependencies
* Twig library dependency
@mmoreram
Copy link
Contributor Author

ping @javiereguiluz Updated & Sorted.

@Pierstoval
Copy link
Contributor

👍

@javiereguiluz
Copy link
Collaborator

Awesome! @mmoreram thanks for woring on this and for dealing so patiently with my peculiarities ;)

@javiereguiluz javiereguiluz merged commit d2d0364 into EasyCorp:master Jan 21, 2015
javiereguiluz added a commit that referenced this pull request Jan 21, 2015
This PR was merged into the master branch.

Discussion
----------

Added project dependencies in composer

* Symfony dependencies (Components and Bundles)
* FrameworkExtraBundle dependencies

Commits
-------

d2d0364 Added project dependencies in composer
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