-
Notifications
You must be signed in to change notification settings - Fork 559
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
Suggested changes to kneth/realm-core-14.0.1
#6516
Conversation
@@ -146,7 +146,7 @@ export enum LogCategory { | |||
/** | |||
* Type for `Realm.setLogLevel` | |||
*/ | |||
export type LogArgs = { | |||
export type LogOptions = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We tend to use "options" to describe the type of object passed as the sole argument to a function.
/** | ||
* Represents an entry in the log. | ||
*/ | ||
export type LogEntry = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe "entry" is semantically more precise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it reads better
* @param category - The category (origin) of the log entry. | ||
* @param level - The level of the log entry. | ||
* @param message - The message of the log entry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As these are not separate arguments to the callback, I've moved these descriptions into the properties on the object's type above.
What, How & Why?
Instead of a lot of suggested changes in the GitHub PR UI, I thought a PR would be better.