-
Notifications
You must be signed in to change notification settings - Fork 47
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
Add a couple named types for BsConfig #716
base: master
Are you sure you want to change the base?
Add a couple named types for BsConfig #716
Conversation
export interface BsConfigFileEntry { | ||
src: string | string[]; | ||
dest?: string; | ||
} | ||
|
||
export type BsConfigFileEntryOrShortcut = string | BsConfigFileEntry; | ||
|
||
export interface BsConfigDiagnosticFilter { | ||
src?: string; | ||
codes?: Array<number | string>; | ||
} | ||
|
||
export type BsConfigDiagnosticFilterOrShortcut = string | number | BsConfigDiagnosticFilter; | ||
|
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.
Let's drop the BsConfig
prefix. If a plugin needs to prefix these, they can do that with the import { FileEntry as BsConfigFileEntry } from 'brighterscript'
syntax.
Also, for a while now I've been wanting the term FileEntry
to represent a single { src: string; dest: string; }
object. BrighterScript currently has an interface named FileObj
for that, which is a terrible name. In roku-deploy
, we already have a definition for your BsConfigFileEntry
interface here, and then the { src: string; dest: string; }
interface is named StandardizedFileEntry
, which is also a terrible name. I'd love to come up with good names for those two, and use them across all the RokuCommunity projects.
Any thoughts on a better name?
export interface FileEntry {
src: string;
dest: string;
}
//what should this be named? FilePathOptions? FilesEntry? MultiFileEntry?
export interface ComplexFileEntry {
src: string | string[];
dest?: 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.
The FileEntry
makes sense to me given that it's a single file path and its destination (no globs).
The second is usually made of globs so you could make that explicit with FileGlobEntry
or FileGlobMapping
(to turn a FileGlobEntry
into a FileEntry[]
, it's clear you need to resolve globs).
You could opt instead to make it clear the second one is an input... FileEntryInput
or FileEntryConfig
, something like that.
Naming is hard 😆
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.
Hmm, what about FileMap
and FileEntry
? Is that too similar?
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.
@elliot-nelson I haven't forgotten about this. I've been busy with the roku conference presentation, and naming is still hard. @chrisdp and I had a talk about some of these today, but still haven't landed on the final naming.
ebfb3e5
to
e98ca6d
Compare
SUMMARY
BsConfigFileEntry
BsConfigFileEntryOrShortcut
BsConfigDiagnosticFilter
BsConfigDiagnosticFilerOrShortcut
DETAILS
When writing a bsc plugin, it can get awkward to write functions that interface with entries that have type algebra (you're copying inline into your own plugin code).
I'm putting my thumb on the scale with these names by suggesting the "real" interface for
BsConfigFileEntry
andBsConfigDiagnosticFilter
, but also providing a type for the shortcutting versions. You could imagine a plugin doing some transforms like so:(Open to a different naming scheme! Just sharing my thought process.)
TESTING