-
Notifications
You must be signed in to change notification settings - Fork 19
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
Upgrade typescript & other packages #157
Conversation
… old behavior, add 2 more tests that look for new behavior
Co-authored-by: Bronley Plumb <[email protected]>
# Conflicts: # src/RokuDeploy.ts
# Conflicts: # src/util.ts
# Conflicts: # src/RokuDeploy.ts # src/cli.ts # src/commands/DeleteInstalledChannelCommand.ts # src/commands/GetDeviceInfoCommand.ts # src/commands/PrepublishCommand.ts # src/commands/PublishCommand.ts # src/commands/SignExistingPackageCommand.ts # src/commands/TakeScreenshotCommand.ts # src/commands/ZipPackageCommand.ts # src/index.ts # src/util.spec.ts # src/util.ts
# Conflicts: # src/index.ts
…comments, delete unnecessary command/tests/options, partially fix exec command
…ms' into upgrade-typescript
…oalescing set to false, propert results should be set as a class property, optional chaining, and no redundant types
# Conflicts: # src/RokuDeploy.spec.ts
src/RokuDeploy.spec.ts
Outdated
import * as errors from './Errors'; | ||
import { util, standardizePath as s, standardizePathPosix as sp } from './util'; | ||
import type { FileEntry, RokuDeployOptions } from './RokuDeployOptions'; | ||
import { cwd, expectPathExists, expectPathNotExists, expectThrowsAsync, outDir, rootDir, stagingDir, tempDir, writeFiles } from './testUtils.spec'; | ||
import { createSandbox } from 'sinon'; | ||
import * as r from 'postman-request'; | ||
import type * as requestType from 'request'; | ||
import type { CaptureScreenshotOptions, ConvertToSquashfsOptions, CreateSignedPackageOptions, DeleteDevChannelOptions, GetDevIdOptions, GetDeviceInfoOptions, RekeyDeviceOptions, SendKeyEventOptions, SideloadOptions } from './RokuDeploy'; | ||
import { RokuDeploy, type CaptureScreenshotOptions, type ConvertToSquashfsOptions, type CreateSignedPackageOptions, type DeleteDevChannelOptions, type GetDevIdOptions, type GetDeviceInfoOptions, type RekeyDeviceOptions, type SendKeyEventOptions, type SideloadOptions } from './RokuDeploy'; |
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 revert this one. I prefer having 1 "import {notTypeStuff}" and then another line for "import type {typeStuff}". Freel free to tweak any eslint rules that complain about 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.
The only thing is this no duplicate import rule doesn't have an option to exclude this scenario and to disable it means it won't create an error if we import from the same file multiple times, which I think that is a good error to have right?
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 just disabled it for the one line
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.
This looks great. Just one issue with the eslint-disable comment, then this is good to go
Fixes #147