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.

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

Console log #284

Merged
merged 6 commits into from
Aug 18, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
15 changes: 0 additions & 15 deletions packages/cli/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions packages/cli/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@
*/

import {Cli} from './lib/Cli';
import {Log} from '@bubblewrap/core';
import {Log, ConsoleLog} from '@bubblewrap/core';

module.exports = async (): Promise<void> => {
const cli = new Cli();
const log = new Log('cli');
const log: Log = new ConsoleLog('cli');
chenlevy24 marked this conversation as resolved.
Show resolved Hide resolved
const args = process.argv.slice(2);

let success;
Expand Down
6 changes: 3 additions & 3 deletions packages/cli/src/lib/cmds/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

import {AndroidSdkTools, Config, DigitalAssetLinks, GradleWrapper, JdkHelper, KeyTool, Log,
TwaManifest, JarSigner, SigningKeyInfo, Result} from '@bubblewrap/core';
ConsoleLog, TwaManifest, JarSigner, SigningKeyInfo, Result} from '@bubblewrap/core';
import * as path from 'path';
import * as fs from 'fs';
import {enUS as messages} from '../strings';
Expand All @@ -38,7 +38,7 @@ class Build {
private jarSigner: JarSigner;

constructor(private config: Config, private args: ParsedArgs,
private log = new Log('build'), private prompt: Prompt = new InquirerPrompt()) {
private log: Log = new ConsoleLog('build'), private prompt: Prompt = new InquirerPrompt()) {
this.jdkHelper = new JdkHelper(process, this.config);
this.androidSdkTools = new AndroidSdkTools(process, this.config, this.jdkHelper, this.log);
this.keyTool = new KeyTool(this.jdkHelper, this.log);
Expand Down Expand Up @@ -191,7 +191,7 @@ class Build {
}

export async function build(config: Config, args: ParsedArgs,
log = new Log('build'), prompt: Prompt = new InquirerPrompt()): Promise<boolean> {
log: Log = new ConsoleLog('build'), prompt: Prompt = new InquirerPrompt()): Promise<boolean> {
const build = new Build(config, args, log, prompt);
return build.build();
}
4 changes: 2 additions & 2 deletions packages/cli/src/lib/cmds/help.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import {Log} from '@bubblewrap/core';
import {Log, ConsoleLog} from '@bubblewrap/core';
import {ParsedArgs} from 'minimist';

const HELP_MESSAGES = new Map<string, string>(
Expand Down Expand Up @@ -87,7 +87,7 @@ const HELP_MESSAGES = new Map<string, string>(
],
);

export async function help(args: ParsedArgs, log = new Log('help')): Promise<boolean> {
export async function help(args: ParsedArgs, log: Log = new ConsoleLog('help')): Promise<boolean> {
// minimist uses an `_` object to store details.
const command = args._[1];
const message = HELP_MESSAGES.get(command) || HELP_MESSAGES.get('main');
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/lib/cmds/install.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import {AndroidSdkTools, Config, JdkHelper, Log} from '@bubblewrap/core';
import {AndroidSdkTools, Config, JdkHelper, Log, ConsoleLog} from '@bubblewrap/core';
import {ParsedArgs} from 'minimist';

const APK_FILE_PARAM = '--apkFile';
Expand All @@ -23,7 +23,7 @@ const DEFAULT_APK_FILE = './app-release-signed.apk';
const PARAMETERS_TO_IGNORE = ['--verbose', '-r'];

export async function install(
args: ParsedArgs, config: Config, log = new Log('install')): Promise<boolean> {
args: ParsedArgs, config: Config, log: Log = new ConsoleLog('install')): Promise<boolean> {
const jdkHelper = new JdkHelper(process, config);
const androidSdkTools = new AndroidSdkTools(process, config, jdkHelper, log);
const apkFile = args.apkFile || DEFAULT_APK_FILE;
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/lib/cmds/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@
*/

import {PwaValidator} from '@bubblewrap/validator';
import {Log} from '@bubblewrap/core';
import {Log, ConsoleLog} from '@bubblewrap/core';
import {ParsedArgs} from 'minimist';
import {printValidationResult} from '../pwaValidationHelper';

const log = new Log('validate');
const log: Log = new ConsoleLog('validate');

/**
* Runs the PwaValidator to check a given URL agains the Quality criteria. More information on the
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/lib/cmds/version.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@

import * as fs from 'fs';
import * as path from 'path';
import {Log} from '@bubblewrap/core';
import {Log, ConsoleLog} from '@bubblewrap/core';

export async function version(log = new Log('version')): Promise<boolean> {
export async function version(log: Log = new ConsoleLog('version')): Promise<boolean> {
const packageJsonFile = path.join(__dirname, '../../../package.json');
const packageJsonContents = await (await fs.promises.readFile(packageJsonFile)).toString();
const packageJson = JSON.parse(packageJsonContents);
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/lib/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

import {join} from 'path';
import {homedir} from 'os';
import {Config, Log} from '@bubblewrap/core';
import {Config, Log, ConsoleLog} from '@bubblewrap/core';
import * as inquirer from 'inquirer';
import {existsSync} from 'fs';
import {promises as fsPromises} from 'fs';
Expand All @@ -44,7 +44,7 @@ async function createConfig(): Promise<Config> {
return new Config(result.jdkPath, result.androidSdkPath);
}

async function renameConfigIfNeeded(log = new Log('config')): Promise<void> {
async function renameConfigIfNeeded(log: Log = new ConsoleLog('config')): Promise<void> {
if (existsSync(DEFAULT_CONFIG_FILE_PATH)) return;
// No new named config file found.
if (!existsSync(LEGACY_CONFIG_FILE_PATH)) return;
Expand Down
8 changes: 2 additions & 6 deletions packages/core/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
"@types/extract-zip": "^1.6.2",
"@types/inquirer": "^6.5.0",
"@types/lodash.template": "^4.5.0",
"@types/mime-types": "^2.1.0",
"@types/node": "^12.12.50",
"@types/node-fetch": "^2.5.7",
"@types/tar": "^4.0.3",
Expand All @@ -47,5 +46,8 @@
"node-fetch": "^2.6.0",
"tar": "^6.0.2",
"valid-url": "^1.0.9"
},
"devDependencies": {
"@types/mime-types": "^2.1.0"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think @andreban made a change that moved all the dev dependencies somewhere - ad139d1 .

I'm not sure if this goes against that, but lets see what he says.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you revert this change, please. mime-types is needed as a runtime dependency. This is also downgrading the minimum version from 2.1.27 to 2.1.0. Generally, devDependencies should be empty in core, cli and validator and listed on the root package.json.

}
}
3 changes: 2 additions & 1 deletion packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import {AndroidSdkTools} from './lib/androidSdk/AndroidSdkTools';
import {Config} from './lib/Config';
import {GradleWrapper} from './lib/GradleWrapper';
import Log from './lib/Log';
import {Log, ConsoleLog} from './lib/Log';
import {JarSigner} from './lib/jdk/JarSigner';
import {JdkHelper} from './lib/jdk/JdkHelper';
import {KeyTool} from './lib/jdk/KeyTool';
Expand All @@ -36,6 +36,7 @@ export {AndroidSdkTools,
JdkHelper,
KeyTool,
Log,
ConsoleLog,
TwaGenerator,
TwaManifest,
DisplayMode,
Expand Down
43 changes: 39 additions & 4 deletions packages/core/src/lib/Log.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2019 Google Inc. All Rights Reserved.
* Copyright 2020 Google Inc. All Rights Reserved.
chenlevy24 marked this conversation as resolved.
Show resolved Hide resolved
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -14,10 +14,45 @@
* limitations under the License.
*/

/**
* An interface for loggers.
*/
export interface Log {

verbose: boolean;

/**
* Prints a debug message to the Log. message is ignored if the Log is not set to verbose.
* @param message the message the be printed.
* @param args extra arguments for the console.
*/
debug(message: string, ...args: string[]): void;

/**
* Prints an info message to the Log. message is ignored if the Log is not set to verbose.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These comments seem to be wrong (the bit about verbose only applies to debug I think).

Also, capital letters at the start of sentences please.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't write it, and I think it is right, if verbose == false it will not print anything

* @param message the message the be printed.
* @param args extra arguments for the console.
*/
info(message: string, ...args: string[]): void;

/**
* Prints an warning message to the Log. message is ignored if the Log is not set to verbose.
* @param message the message the be printed.
* @param args extra arguments for the console.
*/
warn(message: string, ...args: string[]): void;
/**
* Prints an error message to the Log. message is ignored if the Log is not set to verbose.
* @param message the message the be printed.
* @param args extra arguments for the console.
*/
error(message: string, ...args: string[]): void;
};

/**
* An utility class to print nice Log messages.
*/
export default class Log {
export class ConsoleLog implements Log {
private tag: string;
private prefix: string;
private output: Console;
Expand Down Expand Up @@ -85,11 +120,11 @@ export default class Log {
* Creates a new Log using the same output and verbositity of the current Log.
* @param newTag the tag the be used on the new Log instance.
*/
newLog(newTag: string): Log {
newLog(newTag: string): ConsoleLog {
if (this.tag) {
newTag = this.tag + ' ' + newTag;
}
return new Log(newTag, this.verbose, this.output);
return new ConsoleLog(newTag, this.verbose, this.output);
}

private log(fn: Function, message: string, ...args: string[]): void {
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/lib/TwaGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {template} from 'lodash';
import {promisify} from 'util';
import {TwaManifest} from './TwaManifest';
import {ShortcutInfo} from './ShortcutInfo';
import Log from './Log';
import {Log, ConsoleLog} from './Log';
import {ImageHelper, IconDefinition} from './ImageHelper';

const COPY_FILE_LIST = [
Expand Down Expand Up @@ -158,7 +158,7 @@ class Progress {
export class TwaGenerator {
private imageHelper = new ImageHelper();

constructor(private log = new Log('twa-generator')) {}
constructor(private log: Log = new ConsoleLog('twa-generator')) {}

// Ensures targetDir exists and copies a file from sourceDir to target dir.
private async copyStaticFile(
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/lib/TwaManifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import * as fs from 'fs';
import fetch from 'node-fetch';
import {findSuitableIcon, generatePackageId, validateNotEmpty} from './util';
import Color = require('color');
import Log from './Log';
import {Log, ConsoleLog} from './Log';
import {WebManifestIcon, WebManifestJson} from './types/WebManifest';
import {ShortcutInfo} from './ShortcutInfo';

Expand Down Expand Up @@ -108,7 +108,7 @@ export class TwaManifest {
fallbackType: FallbackType;
enableSiteSettingsShortcut: boolean;

private static log: Log = new Log('twa-manifest');
private static log: Log = new ConsoleLog('twa-manifest');

constructor(data: TwaManifestJson) {
this.packageId = data.packageId;
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/lib/androidSdk/AndroidSdkTools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import * as path from 'path';
import util = require('../util');
import {Config} from '../Config';
import {JdkHelper} from '../jdk/JdkHelper';
import Log from '../../lib/Log';
import {Log, ConsoleLog} from '../../lib/Log';

const BUILD_TOOLS_VERSION = '29.0.2';

Expand All @@ -40,7 +40,7 @@ export class AndroidSdkTools {
* @param {jdkHelper} jdkHelper the JDK information to be used by the Android SDK
*/
constructor(process: NodeJS.Process, config: Config, jdkHelper: JdkHelper,
readonly log = new Log('AndroidSdkTools')) {
readonly log: Log = new ConsoleLog('AndroidSdkTools')) {
if (!fs.existsSync(config.androidSdkPath)) {
throw new Error(`androidSdkPath does not exist: ${config.androidSdkPath}`);
}
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/lib/jdk/KeyTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import {existsSync, promises} from 'fs';
import {execute} from '../util';
import {JdkHelper} from './JdkHelper';
import Log from '../Log';
import {Log, ConsoleLog} from '../Log';

export interface KeyInfo {
fingerprints: Map<string, string>;
Expand All @@ -44,7 +44,7 @@ export class KeyTool {
private jdkHelper: JdkHelper;
private log: Log;

constructor(jdkHelper: JdkHelper, log = new Log('keytool')) {
constructor(jdkHelper: JdkHelper, log: Log = new ConsoleLog('keytool')) {
this.jdkHelper = jdkHelper;
this.log = log;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/lib/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {promisify} from 'util';
import {exec, execFile, spawn} from 'child_process';
import {x as extractTar} from 'tar';
import {WebManifestIcon} from './types/WebManifest';
import Log from './Log';
import {Log} from './Log';
import {lookup} from 'mime-types';

const execPromise = promisify(exec);
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/spec/lib/androidSdk/AndroidSdkToolsSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {JdkHelper} from '../../../lib/jdk/JdkHelper';
import {AndroidSdkTools} from '../../../lib/androidSdk/AndroidSdkTools';
import util = require('../../../lib/util');
import * as fs from 'fs';
import {Log} from '../../..';
import {Log, ConsoleLog} from '../../..';

function buildMockConfig(platform: string): Config {
if (platform === 'linux' || platform == 'darwin') {
Expand Down Expand Up @@ -179,7 +179,7 @@ describe('AndroidSdkTools', () => {
const config = buildMockConfig(test.platform);
const process = buildMockProcess(test.platform);
const jdkHelper = new JdkHelper(process, config);
const log = new Log('test');
const log: Log = new ConsoleLog('test');
const androidSdkTools = new AndroidSdkTools(process, config, jdkHelper, log);
spyOn(util, 'execute').and.stub();
await androidSdkTools.install('app-release-signed.apk');
Expand Down Expand Up @@ -244,7 +244,7 @@ describe('AndroidSdkTools', () => {
const config = buildMockConfig(test.platform);
const process = buildMockProcess(test.platform);
const jdkHelper = new JdkHelper(process, config);
const log = new Log('test');
const log: Log = new ConsoleLog('test');
const androidSdkTools = new AndroidSdkTools(process, config, jdkHelper, log);
spyOn(util, 'executeFile').and.stub();
await androidSdkTools.apksigner(
Expand Down