Skip to content
This repository has been archived by the owner on Sep 16, 2021. It is now read-only.

[WIP] work on menu bundle upgrade #206

Closed
wants to merge 5 commits into from

Conversation

cordoval
Copy link
Contributor

Q A
Bug Fix? n
New Feature? n
BC Breaks? n
Deprecations? n
Tests Pass? n
Fixed Tickets
License MIT
Doc PR

Sent using Gush

@cordoval cordoval changed the title bootstrap changes [WIP] work on menu bundle upgrade Aug 20, 2014
- 5.3
- 5.4
# - 5.3
# - 5.4
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 if you would not change this in your commits - we should see if there happen to be problems with different versions of php / symfony.

its not hard to run the tests locally for fast feedback, just look at the .travis.yml and do the same steps.

@@ -0,0 +1,3 @@
adapter: github
issue-tracker: github

Copy link
Member

Choose a reason for hiding this comment

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

we are not going to use gush...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

try to speak for yourself, in view of such a hostile env i am closing the PRs, maybe i will try later

Copy link
Member

Choose a reason for hiding this comment

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

I'm sad that you closed this PR because of this one little thing. It contained lots of nice changes.

In fact, there were just 2 problems with this PR (in a PR with 20+ changes):

  • Some lost comments, which are quite normal since it's a WIP PR
  • You introduced a new tool. When introducing a new tool to a big open source project, you can be sure there will be a discussion with some hard opinions.

Copy link

Choose a reason for hiding this comment

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

I don’t think there was anything wrong with @wouterj’s comment. He part of the CMF core team and just expressed that he doesn’t want to introduce a new tool to the project. What’s wrong with that?

@cordoval cordoval closed this Aug 29, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants