Skip to content

Added recipe for FOSUserBundle (#345 #270) #363

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

Closed

Conversation

patrickbussmann
Copy link

Q A
License MIT

I checked the other pull requests #345 and #270 and combined everything with the ORM version. (entities etc.)

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request passes validation.

Copy link
Member

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

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

Thank you for this.

Please also make sure that Travis is happy.

db_driver: orm # other valid values are 'mongodb', 'couchdb' and 'propel'
firewall_name: main
user_class: AppBundle\Entity\User
# use_listener: true
Copy link
Member

Choose a reason for hiding this comment

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

Remove comments like this.

* @ORM\GeneratedValue()
* @ORM\Column(type="integer")
*/
protected $id;
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer private here. No mean to use protected since nothing extends this class.


public function __construct()
{
parent::__construct();
Copy link
Member

Choose a reason for hiding this comment

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

Maybe write a comment saying that this line is important.

// * @return User[] Returns an array of User objects
// */
/*
public function findByExampleField($value)
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 removed.

I would prefer to see a link to the docs.

@OskarStark
Copy link
Contributor

Any news here?

@patrickbussmann
Copy link
Author

patrickbussmann commented Jun 21, 2018 via email

@OskarStark
Copy link
Contributor

No because of the missing removal of the dependencies which is not possible.

Ok, in this case, please close this PR, thank you!

@maxhelias maxhelias mentioned this pull request Aug 15, 2018
@fabpot
Copy link
Member

fabpot commented Sep 3, 2018

Closing as a duplicate (#270)

@fabpot fabpot closed this Sep 3, 2018
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.

4 participants