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

Bump version #133

Merged
merged 12 commits into from
Dec 18, 2016
Merged

Bump version #133

merged 12 commits into from
Dec 18, 2016

Conversation

scott-w
Copy link
Member

@scott-w scott-w commented Nov 6, 2016

I've bumped the version number and added tested support for Lodash.

This bumps a fair few dependencies that I needed to make this build work. I'd appreciate a bit of a look over this to see if there's anything we need to pick up on. My feeling is we should roll a v1.0.0 release and try to keep this working alongside Mn 3.x if and when we're able to.

Related Issues #131 #132

Scott Walton added 3 commits November 6, 2016 14:08
Copy link
Member

@denar90 denar90 left a comment

Choose a reason for hiding this comment

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

@scott-w will you add build files in another PR?

@scott-w
Copy link
Member Author

scott-w commented Nov 27, 2016

Finally got all the deps updated so Travis can build+test. Yay for trial and error!

Copy link
Contributor

@blikblum blikblum left a comment

Choose a reason for hiding this comment

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

Thanks. Without contextify the travis addons is not necessary but letting them will not hurt

"grunt-cli": "^0.1.13",
"chai": "^3.5.0",
"chai-jquery": "^2.0.0",
"contextify": "^0.1.15",
Copy link
Contributor

Choose a reason for hiding this comment

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

contextify is required?
In my working copy, i upgraded mocha and jsdom alone and worked without contextify dependency

@@ -1,6 +1,6 @@
{
"name": "backbone.syphon",
"version": "0.6.3",
"version": "1.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

I would not bump to 1.0 at least until decide about #125

Copy link
Member Author

Choose a reason for hiding this comment

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

Feeling is that Syphon has depended on jQuery for so long now that changing it for v1 would potentially break code and assumptions.

I do agree that it would be nicer to remove the dependencies going forward. I do see a lot of code transitioning away from jQuery over time.

Perhaps we resolve to remove jQuery for v2?

Copy link
Member Author

Choose a reason for hiding this comment

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

That said, if that's the direction we look to go, I think I should re-add support for jQuery 2.x as it's still widely used and supported.

Copy link
Contributor

Choose a reason for hiding this comment

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

According to semver, breaking changes can occur before 1.0 at will.

If we release, e.g., 0.7 we have the freedom to decide upfront although since the jQuery usage is pretty basic, i believe that will have no regressions

Another thing that i'd like to propose is putting the dependencies as peerDependencies like we do for Marionette, not sure this will be configured as a breaking change

I think that jQuery 1 and 2 should be supported since there's no reason to not support them

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good, I'll make those changes as soon as I get chance.

@scott-w
Copy link
Member Author

scott-w commented Dec 17, 2016

@blikblum I've updated to allow a range of jQuery deps and dropped the package version to 0.7.0. Can you check I've got the bower.json package version correct?

@blikblum
Copy link
Contributor

I don't know about bower.
CI is failing with EPeerInvalid in node v4: The package jquery@3.1.1 does not satisfy its siblings' peerDependencies requirements!

While building another library i learned that is necessary to put the peerDependencies in devDependencies to work.

@blikblum
Copy link
Contributor

LGTM. I think we can ship it

@scott-w scott-w merged commit 0b40452 into marionettejs:master Dec 18, 2016
@scott-w scott-w deleted the bump-version branch December 18, 2016 17:13
@scott-w
Copy link
Member Author

scott-w commented Dec 18, 2016

:shipit:

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.

None yet

5 participants