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

Forced failing unit test for #247 #281

Merged
merged 20 commits into from
Feb 6, 2014
Merged

Forced failing unit test for #247 #281

merged 20 commits into from
Feb 6, 2014

Conversation

mpalourdio
Copy link
Contributor

This small test is made to actually fail, regarding #247.
Merged @mjhca fix too.

@@ -125,7 +125,9 @@ public function getFormSpecification($entity)
protected function checkForExcludeElementFromMetadata(ClassMetadata $metadata, $name)
{
$params = array('metadata' => $metadata, 'name' => $name);
$test = function($r) { return (true === $r); };
$test = function ($r) {
return (true === $r);
Copy link
Member

Choose a reason for hiding this comment

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

No need for ( and )

@UFOMelkor UFOMelkor mentioned this pull request Nov 16, 2013
@Ocramius
Copy link
Member

@mpalourdio could you rebase this one?

@mpalourdio
Copy link
Contributor Author

I just have a few question before pushing. Here is what I do to rebase : (after checking out my Fix247 branch)

  1. git pull --rebase upstream master
  2. resolve conflicts
  3. git rebase --continue.

At this point, if I try to push, i am asked if I want to rebase (again?) or merge before pushing. What am I supposed to do not to screw all commits ?

  • rebase as asked ?
  • merge as asked ?
  • git push --force ?

Please let me know.

@Ocramius
Copy link
Member

Don't ever merge. This is how I usually do it:

git checkout master
git pull --ff-only upstream master #please use only with --ff-only
git checkout my-working-branch
git rebase master

@mpalourdio
Copy link
Contributor Author

I have done as you purposed, but same message. So i rebased as asked and pushed, instead of merge. Hope it's ok.

@Ocramius
Copy link
Member

@mpalourdio ah, I see - rebase gone wrong :-( I'll pull locally and check

@mpalourdio
Copy link
Contributor Author

erf, we crossed commit/comment. Ok, let you see. I can reintroduce my initial commits in a separate branch if it's easier for you

@Ocramius
Copy link
Member

@mpalourdio can work, though I could probably fix it when I have time.

@mpalourdio
Copy link
Contributor Author

ok, let me know if you need a clean PR.

@Ocramius
Copy link
Member

@mpalourdio if you can do that, that'd be easier - not on an awesome connection here :(


if (!isset($elementSpec['spec']['options']['empty_option'])) {
if ((isset($elementSpec['spec']['options']) &&
!array_key_exists('empty_option', $elementSpec['spec']['options'])) ||
Copy link
Member

Choose a reason for hiding this comment

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

Alignment seems to be a bit scrambled here

@Ocramius Ocramius added this to the 0.9.0 milestone Feb 6, 2014
Ocramius added a commit that referenced this pull request Feb 6, 2014
Forced failing unit test for #247
@Ocramius Ocramius merged commit 1c8dd83 into doctrine:master Feb 6, 2014
@mpalourdio mpalourdio deleted the Fix247 branch February 7, 2014 14:19
@gianarb gianarb mentioned this pull request Apr 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BC Break Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants