Proposed resolution on the "change booleans to enums" issue

Over the last few weeks, we’ve had a long drawn out discussion based
around the following github issues and PRs:

- PR #466 “Use an enum to describe directionality of RTP stream”
- PR #467 “Use enum for voiceActivityDetection”
- PR #471 “Use enum for RTCDataChannal’s ordered attribute”

These are based on issue #375, “true as default values for dictionary is
bad practice”.

Among the arguments fielded are:

- Following the WebIDL spec’s advice is a Good Thing in general
- Changing interfaces that people have implemented for aesthetic reasons
is a Bad Thing in general
- Double negatives (disableX = false) is a Bad Thing and should be avoided
- VoiceActivityDetection is such a strange thing, it doesn’t matter if
it’s true or false
- Transceiver direction needs to be 1:1 mapped to SDP direction, so an
enum is fine there (rather than two default-true bools)
- Transceiver shouldn’t be more SDP-like than necessary, so two
direction variables is better.

Settled issues:
- SupportedCapabilties uses true as a default, but that’s a special
case, and we won’t change that.

Positions taken:

>From PR discussions:

- pthatcher: Doesn’t mind too much either way, but would like to see
existing code continue to work (do the “or boolean” thing).
- hta: doesn’t like changing existing implemented APIs without enough
reason. Breaking compatibility is bad.
- martin: thinks directionality change looks good (371), no comments
elsewhere.

>From the public-mediacapture mailing list, Feb 17 - March 3, subject
line “Using enums to avoid default true…”


- Adam Bergkvist: Flip the attribute names in #467 and #471, PR
improvement in #466. Argues for “disableVoiceActivityDetection” in
particular.
- Peter Thatcher: Double negaties are bad. Is change worth it?
- Martin Thomson: The guidance is reasonable - “exists for a reason”
- Jan-Ivar Bruarøy: Not following WebIDL advice is hazardous

Conclusions to be drawn:

- I don’t think we have consensus for introducing double negatives.
They’re out. But “ordered -> order” seems like it could be an acceptable
rename.
- There’s not much of a majority opinion towards using enums (Martin &
Jan-Ivar strong).
- The use of the “or Boolean” trick makes changing to enum more
palatable to those who dislike backwards-incompatible changes
- The “ordered” attribute in datachannels needs to follow the parameter

Proposed plan of action:

- There seems to be an established base for voiceActivityDetection and
the Ordered attribute, and not a clear consensus for change. I suggest
we DO NOT CHANGE these.

- There is no clear consensus for changing the directionality attribute
for RTP streams, but here we have no deployed base, and letting
RTPTransceiver mirror the SDP semantics of the direction attribute more
closely seems like a Good Thing.
I suggest we DO CHANGE this one.

If people don’t indicate that they have strong objections to this
resolution by Wednesday, April 27, the chairs will instruct the editors
to implement this decision.

Received on Tuesday, 19 April 2016 16:51:33 UTC