-
Notifications
You must be signed in to change notification settings - Fork 10
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
TS types #27
base: master
Are you sure you want to change the base?
TS types #27
Conversation
types/index.d.ts
Outdated
// https://man7.org/linux/man-pages/man7/systemd.journal-fields.7.html | ||
export type JournalFields = { | ||
message_id?: string, | ||
code_file?: string, | ||
code_line?: string, | ||
code_func?: string, | ||
errno?: string, | ||
invocation_id?: string, | ||
user_invocation_id?: string, | ||
syslog_facility?: string, | ||
syslog_identifier?: string, | ||
syslog_pid?: string, | ||
syslog_timestamp?: string, | ||
syslog_raw?: string, | ||
documentation?: string, | ||
tid?: string, | ||
unit?: string, | ||
user_unit?: string | ||
} |
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.
Sorry, I'm not a TS expert. Does this definition allow for custom fields?
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.
no! it doesn't
we can add a [custom: string]: string
, let me get around that
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.
added on 40e7073, also replaced all ? with Partial
} | ||
|
||
export default class systemd_journald { | ||
constructor(defaultFields: JournalFields); |
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.
defaultFields
is optional and can be omitted. Thus, change it like this?
constructor(defaultFields: JournalFields); | |
constructor(defaultFields?: JournalFields); |
Just guesstimating here :D
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.
my reasoning was that you wouldn't instantiate it and would use the static methods if you didn't want to provide defaults.
if you'd rather it be optional here too I'll accept the change
Woahhh ... sry for not replying. I somehow missed your reaction. Currently I'm on vacation. Please ping me if nothing happened till begin of June. |
added the allowed fields as listed in https://man7.org/linux/man-pages/man7/systemd.journal-fields.7.html minus the message and priority ones which are handled already in log.js