[WIP] 616 #688
[WIP] 616 #688
Conversation
use forEach instead of map when return value is not used (google#614)
Dependencies (google#615)
* dependencies clean-up * types for child_process' spawnSync * types for child_process' spawnSync + options fix * linting
* relative rootDir support * relative rootDir support (untrackedFiles behavior changed) * relative rootDir doc changes
* prettier + sort imports * splitLines types * unused package 'connect' * ucfirst, isOnline types * ellipsize types * redundant package 'fs-copy-file-sync' * removing extra line * // TODO * packages dependencies update * comment fix * fixes * nicer ellipsize typing * better?
* regroup `inquirer` into a single file * fix typo * linting * switch to `find-up` * switch to `find-up` & `strip-bom` * dependencies update * findUp implementation fix * enum accessor fix * fs-extra & typescript dependency fix * linting * dependencies clean-up (again) * non any cast
| @@ -75,7 +74,6 @@ | |||
| "multimatch": "^4.0.0", | |||
| "normalize-newline": "3.0.0", | |||
| "open": "^6.4.0", | |||
| "path": "^0.12.7", | |||
There was a problem hiding this comment.
path & @types/events packages were erroneously added and are not needed
| @@ -7,7 +7,6 @@ import { authorize, getLoggedInEmail } from '../auth'; | |||
| import { FS_OPTIONS } from '../files'; | |||
| import { readManifest } from '../manifest'; | |||
| import { ERROR, LOG, checkIfOnline, hasOauthClientSettings, safeIsOnline } from '../utils'; | |||
| import { google } from 'googleapis'; | |||
There was a problem hiding this comment.
unused import
| @@ -16,10 +15,10 @@ import { google } from 'googleapis'; | |||
| * @param {string?} options.creds The location of credentials file. | |||
| * @param {boolean?} options.status If true, prints who is logged in instead of doing login. | |||
| */ | |||
| export default async (options: { localhost?: boolean; creds?: string, status?: boolean }) => { | |||
| export default async (options: { localhost?: boolean; creds?: string; status?: boolean }) => { | |||
There was a problem hiding this comment.
Changes below are tslintfixes
| @@ -1,11 +1,10 @@ | |||
| import { async } from 'rxjs/internal/scheduler/async'; | |||
There was a problem hiding this comment.
erroneous & unused import
| @@ -50,6 +50,7 @@ export default async (functionName: string, cmd: { nondev: boolean; params: stri | |||
| */ | |||
| async function runFunction(functionName: string, params: string[], scriptId: string, devMode: boolean) { | |||
| try { | |||
| // TODO: @grant what are the issues/risks of not using local auth? | |||
There was a problem hiding this comment.
I am not familiar with the run api. Does it requires a local auth and fails if global auth?
| import fs from 'fs-extra'; | ||
| import mkdirp from 'mkdirp'; | ||
| import multimatch from 'multimatch'; | ||
| import recursive from 'recursive-readdir'; | ||
| import { async } from 'rxjs/internal/scheduler/async'; |
There was a problem hiding this comment.
erroneous & unused import
|
@grant Not sure why is crashed on Node 8 tests. new files:
The other changes are mostly switching for Pending your feedback (take your time) I'll make the requested changes and add specific tests. |
|
@grant approved? |
|
Oops. Didn't mean to approve. Just the commander change looked okay. Looking for a smaller PR. |
|
This PR is not meant to be merged as is. But for you to "preview" what I have in mind and give directions. |
|
Do you want me to open a cleanup PR with the package updates (commander) and the drop of pre node 8 poly fill? |
|
Please create smaller PRs that only do 1 thing. Let's continue conversation in an issue or new small PR. |
Fixes #616
npm run testsucceeds.npm run lintsucceeds.