-
Notifications
You must be signed in to change notification settings - Fork 2k
Conversation
|
@rhutchison this should wait until 0.4.0 is released because we don't need this to release. In fact, I'd rather try to sort out whether we are going to support Grunt or Gulp before doing this, because there is no point in having support for both as we just need to maintain more. |
|
@ilanbiala are you in the gitter chat? This PR is against your 4.0 branch. Should update dependencies and then figure out gulp vs grunt. Some of these dependencies are very old and if you are releasing 4.0, it should be tested against newer libs. |
|
Resolves #612 |
|
@rhutchison we'll get to review this PR soon, no worries. |
This reverts commit 7d8cea1.
|
hi @rhutchison I want to merge this but there's a bit of a mess there with the commit messages like reverts and some other "mistake commits" as well as some space indentation that is wrong (in the package.json file for example) and some spaces that were added but aren't really a change. I don't want to merge with the current commit history, can you please fix this up quickly and squash your commits to logical commits that make sense with the changes? Please hurry doing so as I want to merge this in to master_to_0.4.0 and quickly after merge that one into 0.4.0. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is that dropdown text suppose to be there like that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's how it works with ui-bootstrap.
|
@lirantal package.json is written to by npm --save and --save-dev Please see npm/npm#4718 I updated .editorconfig - Gym@73be794 |
|
@rhutchison with regards to package.json, you re-ordered all of the packages so the diff looks like this now: and it's hard to figure out which package you added, or removed, or just updated the version for because they are all re-organized again in the new commit. @ilanbiala WDYT? |
|
@rhutchison @lirantal I think we should let the package.json re-organising and spacing go trough now and it'll be easier in the future. When working with package.json trough command line, that's what happens. If we don't let this ugly diff go trough here now, people will have it ahead of them when they'll fork and add/remove packages at their version. |
|
@lirantal the package.json reordering makes sense, but right now it definitely makes it harder to diff. I say we update deps using https://david-dm.org/ and then allow npm to format the package.json in a different PR. |
|
@rhutchison I'm not saying that npm isn't doing something logical, I'm saying that we should manually increment the versions based on david-dm and let npm format the package.json properly in a different commit or even PR, because that will simplify reviewing. |
|
Ok then, I'll go ahead ahead and merge this in, we can indeed resolve the package.json later. |


No description provided.