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

feat: Default url option to location.origin in bundle #648

Merged
merged 2 commits into from Dec 4, 2016

Conversation

sdvg
Copy link
Contributor

@sdvg sdvg commented Dec 4, 2016

Closes hoodiehq/hoodie-client#105. Closes #649

This belongs to hoodiehq/hoodie-client#109. The issue required changes in both repositories.

@@ -65,9 +65,19 @@ function buildBundle (config, hoodieClientPath, callback) {
return callback(error)
}

var options = config.url ? '{url: "' + config.url + '"}' : ''
var initBuffer = Buffer('\n\nhoodie = new Hoodie(' + options + ')')
var initScript = '\n\n'
Copy link
Member

Choose a reason for hiding this comment

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

this won’t hurt, but am curious: why not just var initScript = ''?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only to have some space between the client and initialization code. It also was the same way before :)

@gr2m
Copy link
Member

gr2m commented Dec 4, 2016

@sdvg Hoodie Client v6 is released, can you please update the package.json file as part of this PR?

@sdvg
Copy link
Contributor Author

sdvg commented Dec 4, 2016

@gr2m
Copy link
Member

gr2m commented Dec 4, 2016

ooops yes, sorry, we have to wait for v7, currently waiting for travis https://travis-ci.org/hoodiehq/hoodie-client/builds/181153131

@gr2m
Copy link
Member

gr2m commented Dec 4, 2016

@@ -13,7 +13,7 @@
"dependencies": {
"@hoodie/account": "^2.0.0",
"@hoodie/admin": "^1.0.1",
"@hoodie/client": "^6.0.0",
"@hoodie/client": "^7.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

when we update "dependencies", we use the fix: prefix to force a new release, this helps to deduplicate (sub) dependencies. Only for "devDependencies" we use the chore: prefix because they don’t get installed by modules that depend on it.

But for this PR it makes no difference as we already have the feat: prefix, so I’m going ahead and merge it as is :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, will do so next time :)

@gr2m gr2m merged commit 2cd59db into master Dec 4, 2016
@sdvg sdvg deleted the 105-bundle-default-url branch December 4, 2016 19:45
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

2 participants