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

Spelling #4828

Merged
merged 59 commits into from
Sep 11, 2017
Merged

Spelling #4828

merged 59 commits into from
Sep 11, 2017

Conversation

jsoref
Copy link
Contributor

@jsoref jsoref commented Sep 7, 2017

I can file an issue if necessary.

This series can be squashed, but it's much easier to drop words when they're isolated...

Copy link
Contributor

@TimvdLippe TimvdLippe left a comment

Choose a reason for hiding this comment

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

LGTM with 1 small question. Funny to see that the spelling mistakes actually broke some tests as well, glad these are fixed now (and still passing 🎉 )

@@ -477,7 +477,7 @@ Polymer 2.0 will continue to use a [shim](https://github.com/webcomponents/shady
* <a name="breaking-dynamic-functions"> Declaring a method name used as an observer or computing function in the `properties` block causes the method property itself to become a dependency for any effects it is used in, meaning the effect for that method will run whenever the method is set, similar to 1.x. However, due to removing the `undefined` rule noted above, in 2.x if such a method exists when the element is created, it will run with initial values of arguments, even in the case some or all arguments are `undefined`.
* <a name="breaking-binding-notifications">‘notify’ events are no longer fired when value changes _as result of binding from host_, as a major performance optimization over 1.x behavior. Use cases such as `<my-el foo="{{bar}}" on-foo-changed="fooChanged">` are no longer supported. In this case you should simply user a `bar` observer in the host. Use cases such as dynamically adding a `property-changed` event listener on for properties bound by an element's host by an actor other than the host are no longer supported.
* <a name="breaking-properties-deserialization"></a>In order for a property to be deserialized from its attribute, it must be declared in the `properties` metadata object
* <a name="breaking-colleciton"></a>The `Polymer.Collection` and associated key-based path and splice notification for arrays has been eliminated. See [explanation here](https://github.com/Polymer/polymer/pull/3970#issue-178203286) for more details.
* <a name="breaking-collection"></a>The `Polymer.Collection` and associated key-based path and splice notification for arrays has been eliminated. See [explanation here](https://github.com/Polymer/polymer/pull/3970#issue-178203286) for more details.
Copy link
Contributor

Choose a reason for hiding this comment

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

This breaks link to the README. Do we know which pages link to this specific section?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hope by "we", you're asking the polymer team. And because it's an anchor, you won't be able to see it in your web server log file.

I certainly don't know :-( .

The best I can offer are the following:

  • There are no obvious hits.
  • Unlike renaming a file, the worst outcome is that the user is at the top of the page, with the anchor in the urlbar. They could look for "collection" and would probably find their way to the right part of the document.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, the "we" was targeting the Polymer team. It was just a general thought and I am not sure if we can answer this question. Let me ask some others 😄

@jsoref
Copy link
Contributor Author

jsoref commented Sep 7, 2017

wrt. The test for readonlyvalue, I don't really want to think too much about it.

I consider us lucky here :-) ...

I spent a fairly long time trying to debug a typo in a test in mochajs/mocha#2983 -- where correcting the typo changed the behavior of their test (and thus meant they weren't testing what they thought they were testing!).

@dfreedm dfreedm changed the base branch from optional-passive-events to master September 7, 2017 21:19
@dfreedm
Copy link
Member

dfreedm commented Sep 7, 2017

@jsoref awesome work, but you based this on optional-passive-event instead of master, so I "fixed" it.

@dfreedm
Copy link
Member

dfreedm commented Sep 7, 2017

Oh shoot, can you rebase this against master now? I don't want those other commits from optional-passive-events in here.

@jsoref
Copy link
Contributor Author

jsoref commented Sep 7, 2017

Sure, when I get to a computer w/ access to that.
Sorry, picking base branches for development is hard. (Rebasing w/ hg is easy.)

@jsoref
Copy link
Contributor Author

jsoref commented Sep 10, 2017

@azakus : done

Copy link
Member

@dfreedm dfreedm left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@dfreedm dfreedm merged commit 5ab0a86 into Polymer:master Sep 11, 2017
@jsoref jsoref deleted the spelling branch September 12, 2017 15:53
@@ -3512,7 +3512,7 @@ The following notable changes have been made since the 2.0 Preview announcement.

- paper-input element doesn't show keyboard on Firefox OS 2.0 [\#727](https://github.com/Polymer/polymer/issues/727)

- designerr on polymer page lacks demo'd functoniality from youtube quickstart. [\#726](https://github.com/Polymer/polymer/issues/726)

Choose a reason for hiding this comment

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

designerr

Found another one :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants