Skip to content
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

RealmType -> Realm.Type #35

Merged
merged 3 commits into from
Sep 30, 2015
Merged

RealmType -> Realm.Type #35

merged 3 commits into from
Sep 30, 2015

Conversation

alazier
Copy link
Contributor

@alazier alazier commented Sep 29, 2015

First step towards proper namespacing

@appden

@alazier
Copy link
Contributor Author

alazier commented Sep 29, 2015

retest this please

1 similar comment
@jpsim
Copy link
Contributor

jpsim commented Sep 29, 2015

retest this please

@appden
Copy link
Contributor

appden commented Sep 29, 2015

Looks good. I would personally prefer Types (like React.PropTypes), but that isn't something I feel super strongly about. Also, React.PropTypes uses lowercased types like React.PropTypes.string. That's another thing I don't feel that strongly about, but I wanted to point it out since our users will be switching back and forth between defining types in React and Realm, so maybe consistency is worthwhile? If you'd rather not worry about that, then LGTM! 👍

@alazier
Copy link
Contributor Author

alazier commented Sep 29, 2015

I'm also don't feel strongly about different naming schemes but think we should go with the most idiomatic option. Can you think of other libraries that might use other language or formatting? If not I'm happy to copy what React does.

@appden
Copy link
Contributor

appden commented Sep 29, 2015

Nothing else solid is coming to mind. I think we should go lowercase for consistency with React, but also typeof in JS returns lowercased strings for the types.

@appden
Copy link
Contributor

appden commented Sep 29, 2015

Other than that. 👍

@alazier
Copy link
Contributor Author

alazier commented Sep 30, 2015

It looks like the consensus online is to use capitalized names for constants and enums:
http://google-styleguide.googlecode.com/svn/trunk/javascriptguide.xml#Constants

Will go with that for now but can change later if we want.

alazier added a commit that referenced this pull request Sep 30, 2015
RealmType -> Realm.Type
@alazier alazier merged commit 97b8742 into master Sep 30, 2015
@alazier alazier deleted the al-type branch October 26, 2015 19:45
alazier pushed a commit that referenced this pull request Feb 18, 2016
Add headers and core's CPP flags to cmake
alazier pushed a commit that referenced this pull request Sep 14, 2016
@pawellewandowski pawellewandowski mentioned this pull request Apr 14, 2017
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants