Skip to content

api [nfc]: Use Dart-standard camel case throughout our source code #51

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

Merged
merged 3 commits into from
Apr 6, 2023

Conversation

gnprice
Copy link
Member

@gnprice gnprice commented Apr 5, 2023

If the field names and parameter names used in the API bindings match the snake case of the Zulip API, then that will unavoidably propagate to some names found throughout the rest of our code -- as indeed it already has, as seen in this diff. There's no clean line that can be drawn within our source code so that snake-case names go on one side, and camel-case names on the other.

So we'd inevitably have an unpredictable mix of both kinds of names, and often of the same name appearing in two different forms in the same code. That's what we see in the zulip-mobile RN app.

Instead, draw that line at the very boundary of our code, where the data comes and goes via JSON. Happily the json_serializable code generator allows us to largely just write camel case and let the conversion to and from snake case be done automatically.

This PR also makes the same conversion for credential_fixture.dart, which means one will need to update that file. (That's something I don't expect to happen more than once or twice
more before we're able to do away with that mechanism entirely.)

gnprice added 3 commits April 5, 2023 15:20
This will allow us to switch to the Dart standard convention of
camel case within our source code, while still interoperating
smoothly with the snake case in the Zulip API.
If the field names and parameter names used in the API bindings
match the snake case of the Zulip API, then that will unavoidably
propagate to some names found throughout the rest of our code --
as indeed it already has, as seen in this diff.  There's no clean line
that can be drawn within our source code so that snake-case names
go on one side, and camel-case names on the other.

So we'd inevitably have an unpredictable mix of both kinds of names,
and often of the same name appearing in two different forms in the
same code.  That's what we see in the zulip-mobile RN app.

Instead, draw that line at the very boundary of our code, where
the data comes and goes via JSON.  Happily the json_serializable
code generator allows us to largely just write camel case and let
the conversion to and from snake case be done automatically.

Written by removing those ignore_for_file lines, and then using
Android Studio's "rename" feature many times.  Then grepped for
`\w+(_\w+)+` to find stragglers in comments, and fixed those by hand.

There's a couple of remaining stragglers in credential_fixture.dart,
which I'll handle as a followup commit.
This is non-NFC because one needs to update `credential_fixture.dart`.
(That's something I don't expect to happen more than once or twice
more before we're able to do away with that mechanism entirely.)
@chrisbobbe chrisbobbe merged commit bd8e59d into zulip:main Apr 6, 2023
@chrisbobbe
Copy link
Collaborator

Great, thanks! LGTM; merged, and I updated my credential_fixture.dart.

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

Successfully merging this pull request may close these issues.

2 participants