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

Naming inconsistency #11

Closed
jan-ivar opened this Issue Jul 16, 2015 · 8 comments

Comments

Projects
None yet
3 participants
@jan-ivar
Contributor

jan-ivar commented Jul 16, 2015

Looking at RTCStats desendants:

  • RTCRTPStreamStats : RTCStats
  • RTCCodec : RTCStats
  • RTCPeerConnectionStats : RTCStats
  • RTCMediaStreamStats : RTCStats
  • RTCMediaStreamTrackStats : RTCStats
  • RTCDataChannelStats : RTCStats
  • RTCTransportStats : RTCStats
  • RTCIceCandidateAttributes : RTCStats
  • RTCIceCandidatePairStats : RTCStats
  • RTCCertificateStats : RTCStats

They're all "*Stats" except two. Not that it matters terribly since these are dictionaries, but shouldn't we be consistent?

@vr000m

This comment has been minimized.

Show comment
Hide comment
@vr000m

vr000m Jul 16, 2015

Contributor

Perhaps adding *Info instead of *Stats seems more appropriate as these dictionaries do not contain any metrics or statistics.

  • RTCCodecInfo
  • RTCIceCandidateAttributeInfo

OTOH, RTCCertificateStats could be RTCCertificateInfo for the same reason. I do not feel strongly about it.

Contributor

vr000m commented Jul 16, 2015

Perhaps adding *Info instead of *Stats seems more appropriate as these dictionaries do not contain any metrics or statistics.

  • RTCCodecInfo
  • RTCIceCandidateAttributeInfo

OTOH, RTCCertificateStats could be RTCCertificateInfo for the same reason. I do not feel strongly about it.

@jan-ivar

This comment has been minimized.

Show comment
Hide comment
@jan-ivar

jan-ivar Jul 16, 2015

Contributor

@vr000m I follow your reasoning, however, anything returned from getStats is stats IMHO, not attributes, info or attribute-info. Dictionaries may grow over time and add more data, so I for one would rather see our types here reflect inheritance (RTCStats).

Contributor

jan-ivar commented Jul 16, 2015

@vr000m I follow your reasoning, however, anything returned from getStats is stats IMHO, not attributes, info or attribute-info. Dictionaries may grow over time and add more data, so I for one would rather see our types here reflect inheritance (RTCStats).

@alvestrand

This comment has been minimized.

Show comment
Hide comment
@alvestrand

alvestrand Aug 24, 2015

Contributor

My personal leaning would be towards "info" - if one wants to know the inheritance hierarchy, one should walk the prototype chain, not try to parse names.
Names need to be 1) unique and 2) memorable. But I can't bring myself to feel strongly about this.

Contributor

alvestrand commented Aug 24, 2015

My personal leaning would be towards "info" - if one wants to know the inheritance hierarchy, one should walk the prototype chain, not try to parse names.
Names need to be 1) unique and 2) memorable. But I can't bring myself to feel strongly about this.

@jan-ivar

This comment has been minimized.

Show comment
Hide comment
@jan-ivar

jan-ivar Aug 25, 2015

Contributor

@alvestrand dictionaries don't have prototypes. Since there's very little enforcement of this inheritance in JS, I would recommend name consistency here. See also w3c/webrtc-pc#275

Contributor

jan-ivar commented Aug 25, 2015

@alvestrand dictionaries don't have prototypes. Since there's very little enforcement of this inheritance in JS, I would recommend name consistency here. See also w3c/webrtc-pc#275

@alvestrand

This comment has been minimized.

Show comment
Hide comment
@alvestrand

alvestrand Oct 23, 2015

Contributor

If we want to make this a principle, the document needs to state that it is.
I think we should accept the principle.

Contributor

alvestrand commented Oct 23, 2015

If we want to make this a principle, the document needs to state that it is.
I think we should accept the principle.

@alvestrand

This comment has been minimized.

Show comment
Hide comment
@alvestrand

alvestrand Oct 29, 2015

Contributor

Principle seems to be that *Attribute and *Info should go, and *Stats should be added.

Contributor

alvestrand commented Oct 29, 2015

Principle seems to be that *Attribute and *Info should go, and *Stats should be added.

@alvestrand

This comment has been minimized.

Show comment
Hide comment
@alvestrand

alvestrand May 9, 2016

Contributor

RTCIceCandidateAttributes -> RTCIceCandidateStats is done in #36

Contributor

alvestrand commented May 9, 2016

RTCIceCandidateAttributes -> RTCIceCandidateStats is done in #36

@vr000m vr000m assigned alvestrand and unassigned vr000m May 9, 2016

vr000m added a commit that referenced this issue May 23, 2016

@vr000m vr000m added the PR exists label May 23, 2016

@vr000m

This comment has been minimized.

Show comment
Hide comment
@vr000m

vr000m May 24, 2016

Contributor

Closing this issue, since both inconsistencies are now resolved.

Contributor

vr000m commented May 24, 2016

Closing this issue, since both inconsistencies are now resolved.

@vr000m vr000m closed this May 24, 2016

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