Skip to content
This repository was archived by the owner on Sep 15, 2021. It is now read-only.

observe/polymer: Uppercase and refactor annotation names #52

Closed
DartBot opened this issue Jun 5, 2015 · 6 comments
Closed

observe/polymer: Uppercase and refactor annotation names #52

DartBot opened this issue Jun 5, 2015 · 6 comments

Comments

@DartBot
Copy link

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/444270?v=3" align="left" width="96" height="96"hspace="10"> Issue by seaneagan
Originally opened as dart-lang/sdk#15159


issue dart-lang/sdk#0 argued for upper casing the dart:core annotations, but looks like there just wasn't enough time to make that change before 1.0. However, it's not too late to make the same change for observe/polymer:

@observable
@reflectable
@published

->

@observed()
@Reflected()
@published()

and then hopefully later fix issue dart-lang/sdk#2 shortening them to:

@observed
@Reflected
@published

Removing the 'Property' suffix from PublishedProperty and ObservableProperty makes them consistent with the current Reflectable, which could allow them to also be used at the class level to mark all properties as such. And since @­Observable doesn't work, the 'able' suffix doesn't work, but the 'ed' suffix seems to work in all cases, and (FWIW) matches Deprecated.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/1081711?v=3" align="left" width="48" height="48"hspace="10"> Comment by jmesserly


Personally I doubt 13582 will be fixed. Makes this likely moot.

I don't like "observed" because it hasn't necessarily been observed. And may never be. I think "@published" is the outlier. One idea there was to make it @­attribute.


Added Area-Polymer, Triaged labels.
Marked this as being blocked by dart-lang/sdk#13582.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/2049220?v=3" align="left" width="48" height="48"hspace="10"> Comment by sigmundch


added also prerequisite on issue dart-lang/sdk#11474, so we first agree on what the style should be before we change it in polymer.


Marked this as being blocked by dart-lang/sdk#11474.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/1081711?v=3" align="left" width="48" height="48"hspace="10"> Comment by jmesserly


given the convention in corelib already exists, I doubt we'll change this. At least, I'm not seeing a persuasive reason.

(I get that it is familiar from C# and Java, but Dart annotations are full const expressions, so they don't have the same limitations)

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/2049220?v=3" align="left" width="48" height="48"hspace="10"> Comment by sigmundch


Added Library-Observe, Library-Polymer labels.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/3276024?v=3" align="left" width="48" height="48"hspace="10"> Comment by anders-sandholm


Removed Library-Observe label.
Added Pkg-Observe label.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/1081711?v=3" align="left" width="48" height="48"hspace="10"> Comment by jmesserly


not planned to remove the shorthand forms, but if you prefer you can most certainly use the const ctors instead:

@ObservableProperty()
@reflectable()
@PublishedProperty()

(Note that ObservableProperty couldn't be named Observable as it would conflict with the mixin/base class type.)


Added NotPlanned label.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

1 participant