Skip to content

Conversation

@EvaLok
Copy link
Contributor

@EvaLok EvaLok commented Aug 5, 2016

copied the example and was wondering why it wasn't working.. then noticed ;)

@dbu dbu merged commit a79f41e into FriendsOfSymfony:1.4 Aug 5, 2016
@dbu
Copy link
Contributor

dbu commented Aug 5, 2016

ups, thanks for the fix!

@ddeboer
Copy link
Member

ddeboer commented Nov 15, 2016

And then we had this error again on master, cherry-picked: d126f16. 😉

@dbu
Copy link
Contributor

dbu commented Nov 15, 2016

we should be able to merge whole 1.4 into master, no?

@stof
Copy link
Member

stof commented Nov 15, 2016

would be better indeed

@ddeboer
Copy link
Member

ddeboer commented Nov 15, 2016

True, but this was the only change between master and 1.4.

@stof
Copy link
Member

stof commented Nov 15, 2016

@ddeboer a merge is still better, as it means that the same commit is in both branches.
With a cherry pick, we have to manually look in the master history to discover that the patch was also applied, instead of using a comparison view again and seeing that no commits are in 1.4 without being in master.
This is why the Symfony maintenance workflow (used by this bundle as well, until your cherry-pick) is based on merging maintained branches rather than cherry picking.

@ddeboer
Copy link
Member

ddeboer commented Nov 15, 2016

I know, merging would have been better – a slip-up due to simultaneously working on another project where we (necessarily) do cherry-picking instead because of diverging branches.

So, should I merge anyway (and have 2 identical commit messages on master)?

@stof
Copy link
Member

stof commented Nov 15, 2016

I would say yes, as it allows to go back to the merge-based process for this repo (which is much easier to maintain)

@ddeboer
Copy link
Member

ddeboer commented Nov 15, 2016

Okay, just did! Sorry for the confusion.

@dbu
Copy link
Contributor

dbu commented Nov 15, 2016 via email

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