use enum for EventType or assert them inside methods about firebase-admin-interop HOT 5 OPEN

Here is portion of official Dart style guide re: naming constants and enum values:
https://www.dartlang.org/guides/language/effective-dart/style#prefer-using-lowercamelcase-for-constant-names :

In new code, use lowerCamelCase for constant variables, including enum values.

I'd prefer to stick to the style guide as it makes this Dart code consistent with most of the Dart code available on Pub and the Dart SDK itself.

from firebase-admin-interop.

I think enum fits well in this case. Only thing is that we should name values without underscores to comply with Dart style guide, e.g. EventType.childAdded.

Note that it would be a breaking change so would have to be released as 2.0.0 which for such a small change might be hard to justify major version bump.

We can approach this in two steps:

1. Add assertions as per your second suggestions, this would be non breaking. We can also add an abstract class with constants so that users are not forced to hard code strings. E.g.:

abstract class EventType {
  static const String childAdded = 'child_added';
  // ... etc
}

Would be nice to update examples and dartdocs to refer to these constants so that it's easier to discover.

All of this is non-breaking and we can release it in 1.x.x.

2. When we gather enough breaking changes or there is upstream (JS SDK) change that requires us to bump major version to 2.0.0 we can replace the abstract class with an enum and update all method signatures accordingly.

How does this sound?

from firebase-admin-interop.

naming: if we are not using the firebase string name child_added, I suggest considering uppercase low dash (ie. CHILD_ADDED). Maybe it's just a personal preference, though it's also used for firebase.database.ServerValues.TIMESTAMP.

I understand the possible breaking changes, so the two steps are pretty good ideas.

from firebase-admin-interop.

Oh, I see. I was searching for 'constant' and 'enum' but didn't catch it somehow. My bad.

from firebase-admin-interop.

FYI: I added EventType with constants as per discussion and released together with your change of the on method as 1.2.0.

from firebase-admin-interop.