-
Notifications
You must be signed in to change notification settings - Fork 47
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
2.0 preview #94
2.0 preview #94
Conversation
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. |
CLAs look good, thanks! |
7cdd4fa
to
c4f309d
Compare
@@ -189,6 +200,10 @@ | |||
* transaction, or rejects if an unsupported method is attempted. | |||
*/ | |||
transaction: function(method, key, value) { | |||
if (!self.Promise || !this.supportsIndexedDB) { |
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.
Wahhhhhh this repeated if
statement blows, but I don't have a better solution
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.
will remove ifs and console log the error
var BASE_URI = currentScript.ownerDocument.baseURI; | ||
|
||
// base uri workaround for IE and Safari | ||
var BASE_URI = document._currentScript ? document._currentScript.baseURI : |
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.
I think in 2.0
it's currentScript
without the underscore, am I right? Wrong? Not sure, that's certain.
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.
making it certain...
bower.json
Outdated
}, | ||
"ignore": [] | ||
"ignore": [], | ||
"resolutions": { |
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.
Don't think resolutions is needed anymore
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.
not any moreee
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.
Minor comments left -- mostly about figuring out the version for this element :)
// Polymer 2 baseURI polyfill for IE and Safari | ||
if (Polymer.Element && HTMLImports.importForElement) { | ||
BASE_URI = HTMLImports.importForElement(document.currentScript).baseURI; | ||
|
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.
Delinquent new line :)
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.
uhhhhhh....
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.
see commit msg
bower.json
Outdated
@@ -3,7 +3,7 @@ | |||
"authors": [ | |||
"The Polymer Authors" | |||
], | |||
"version": "0.11.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.
let's check with @cdata that the version here shouldn't be 1.0.0
1d81c7a
to
c1b3407
Compare
bower.json
Outdated
@@ -16,21 +16,43 @@ | |||
"homepage": "", | |||
"private": true, | |||
"dependencies": { | |||
"promise-polyfill": "polymerlabs/promise-polyfill#^1.0.0", | |||
"polymer": "polymer/polymer#^1.2.0" | |||
"polymer": "polymer/polymer#2.0.0-rc.3" |
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.
Should this use ^
to get the latest version of Polymer2? Something like this: https://github.com/PolymerElements/app-route/blob/2.0-preview/bower.json#L19
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.
nice catch, @frankiefu
Tests fail in 1.9.0 due to: |
DO NOT MERGE
Major changes:
Promise polyfill moved to dev dependencies