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.

Sign up for GitHub

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

Jump to bottom

Add a couple named types for BsConfig #716

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add a couple named types for BsConfig #716

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4e05aa4
Provide named types for a couple of bsconfig type lines
elliot-nelson Oct 9, 2022
f5c3ca3
consistent prefix
elliot-nelson Oct 9, 2022
21b388a
Merge branch 'master' into enelson/types-for-bsconfig
TwitchBronBron Nov 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions src/BsConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,19 @@
import type { LogLevel } from './Logger';

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;

Comment on lines +3 to +16
Copy link
Member

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;
}

Copy link
Contributor Author

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 😆

Copy link
Member

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?

Copy link
Member

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.

export interface BsConfig {
/**
* The inheritance tree for all parent configs used to generate this config. Do not set this, it is computed.
Expand Down Expand Up @@ -33,7 +47,7 @@ export interface BsConfig {
* If using the {src;dest;} format, you can specify a different destination directory
* for the matched files in src.
*/
files?: Array<string | { src: string | string[]; dest?: string }>;
files?: Array<BsConfigFileEntryOrShortcut>;

/**
* The path where the output zip file should be placed.
Expand Down Expand Up @@ -120,7 +134,7 @@ export interface BsConfig {
/**
* A list of filters used to exclude diagnostics from the output
*/
diagnosticFilters?: Array<number | string | { src: string; codes: (number | string)[] } | { src: string } | { codes: (number | string)[] }>;
diagnosticFilters?: Array<BsConfigDiagnosticFilterOrShortcut>;

/**
* Specify what diagnostic types should be printed to the console. Defaults to 'warn'
Expand Down