Skip to content

Fixing Security Entity Provider tutorial #2765

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
wants to merge 1 commit into from
Closed

Fixing Security Entity Provider tutorial #2765

wants to merge 1 commit into from

Conversation

gondo
Copy link
Contributor

@gondo gondo commented Jun 24, 2013

for more info see #2756
squashed commits from pull request #2753

(based on what i found, the only way how to squash commits in pull request on github is to create another pull request)

@gondo gondo mentioned this pull request Jun 24, 2013
@stof
Copy link
Member

stof commented Jun 24, 2013

FYI, squashing the commit does not require opening a new PR. You can force the push to the same branch.

However, I think you messed your squashing as the commit authorship is now attributed to @weaverryan

@gondo
Copy link
Contributor Author

gondo commented Jun 24, 2013

yes i've seen that and i have no idea hows that possible :) any suggestion how to change author?

@docteurklein
Copy link
Contributor

use git rebase -i

@gondo
Copy link
Contributor Author

gondo commented Jun 24, 2013

ok i've changed the author but git push -f did not change this pull request, is there a way to do it or should i create another pull request?
(sorry for mess guys)

@stof
Copy link
Member

stof commented Jun 24, 2013

without the output of git push -f, it is hard to tell you why the push has not succeeded

@stof
Copy link
Member

stof commented Jun 24, 2013

Actually, looking at your repo, it seems like your foxed commit is in the patch-2 branch. But this PR is from the patch-1 branch

@gondo
Copy link
Contributor Author

gondo commented Jun 24, 2013

ok this should do it

@gondo
Copy link
Contributor Author

gondo commented Jun 24, 2013

i've accidentally took 1 commit from weaverryan but its just some design change as you can see. pls dont make me to change it again :)


Below is an export of my ``User`` table from MySQL with user `admin`
and password `admin`. For details on how to create user records and
encode their password, see :ref:`book-security-encoding-user-password`.
Copy link
Member

Choose a reason for hiding this comment

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

I don't like the first person view here, I think "Below is an export of the user table containing a admin user with password admin." (I'm not quite sure whether or not literals should be used here)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i've just iterated on what was already there. as you can see, i did not change this text just improved it.

Copy link
Member

Choose a reason for hiding this comment

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

I know. I'm sorry if I offended you. I was just wondering if you could change this as well. Otherwise, I can open a new pull request on it after this one is merged. Would be fine with either way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

:) no hard feelings. honestly at this stage i would prefer to get this merged as its taking quite some time. i might do this change later today if this will still not be merged.

Copy link
Member

Choose a reason for hiding this comment

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

@gondo @weaverryan is the only man who merges stuff, it can take a couple weeks before it get merged

@weaverryan
Copy link
Member

Hi @gondo!

Phew, ok! So, you had a lot of very nice changes to this entry. I patched your commit (with a little bit of cleanup) into the 2.2 branch at sha: 1d00f06. I also made tweaks at sha: 7a37651 and sha: c10c405 where I rearranged some of your changes and added even more things to them.

You made a lot of nice improvements to this entry, so thank you very much! But handling them was quite difficult and the changes were large. So, I hope you'll contribute in the future, but if you do, smaller pull requests are much easier for me to handle.

Thanks!

@gondo
Copy link
Contributor Author

gondo commented Jul 1, 2013

thanks. i ll keep that in mind if i ll have further changes. im going through the documentation one by one as im getting familiar with symfony.

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.

6 participants