Skip to content

Commit

Permalink
feat!: cleanup custom logging (#206)
Browse files Browse the repository at this point in the history
This changes the logger option provided to `reviewPullRequest()` and `createPullRequest()` to be a custom `Logger` interface that `code-suggester` declares:

```ts
interface LogFn {
  /* eslint-disable @typescript-eslint/no-explicit-any */
  <T extends object>(obj: T, msg?: string, ...args: any[]): void;
  (msg: string, ...args: any[]): void;
  /* eslint-enable @typescript-eslint/no-explicit-any */
}

export interface Logger {
  error: LogFn;
  warn: LogFn;
  info: LogFn;
  debug: LogFn;
  trace: LogFn;
}
```

* This logger interface can be fulfilled by a default `Pino` logger as well as plain old `console`.
* We now default the logger to a null implementation to keep output to a minimum while allowing for a custom logger if you want output.
* We can drop the `pino` dependency as we now provide a null default logger implementation.

Fixes #178
Fixes #183
  • Loading branch information
chingor13 committed Apr 23, 2021
1 parent 6719b68 commit 3e4df30
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 106 deletions.
91 changes: 0 additions & 91 deletions package-lock.json

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

2 changes: 0 additions & 2 deletions package.json
Expand Up @@ -47,7 +47,6 @@
"diff": "^5.0.0",
"glob": "^7.1.6",
"parse-diff": "^0.8.0",
"pino": "^6.3.2",
"yargs": "^16.0.0"
},
"devDependencies": {
Expand All @@ -58,7 +57,6 @@
"@types/diff": "^5.0.0",
"@types/mocha": "^8.0.0",
"@types/node": "^14.0.20",
"@types/pino": "^6.3.0",
"@types/proxyquire": "^1.3.28",
"@types/sinon": "^10.0.0",
"c8": "^7.0.1",
Expand Down
2 changes: 1 addition & 1 deletion src/bin/handle-git-dir-change.ts
Expand Up @@ -84,7 +84,7 @@ function parseGitDiff(
const relativePath = statusAndPath[1];
return {oldMode, newMode, status, relativePath};
} catch (err) {
logger.warning(
logger.warn(
`\`git diff --raw\` may have changed formats: \n ${gitDiffPattern}`
);
throw err;
Expand Down
6 changes: 3 additions & 3 deletions src/index.ts
Expand Up @@ -22,10 +22,10 @@ import {
FileData,
FileDiffContent,
CreateReviewCommentUserOptions,
Logger,
} from './types';
export {Changes} from './types';
import {Octokit} from '@octokit/rest';
import {Logger, LoggerOptions} from 'pino';
import {logger, setupLogger} from './logger';
import {
addPullRequestDefaults,
Expand Down Expand Up @@ -54,7 +54,7 @@ export async function reviewPullRequest(
octokit: Octokit,
diffContents: Map<string, FileDiffContent> | string,
options: CreateReviewCommentUserOptions,
loggerOption?: Logger | LoggerOptions
loggerOption?: Logger
): Promise<number | null> {
setupLogger(loggerOption);
// if null undefined, or the empty map then no changes have been provided.
Expand Down Expand Up @@ -111,7 +111,7 @@ async function createPullRequest(
octokit: Octokit,
changes: Changes | null | undefined,
options: CreatePullRequestUserOptions,
loggerOption?: Logger | LoggerOptions
loggerOption?: Logger
): Promise<number> {
setupLogger(loggerOption);
// if null undefined, or the empty map then no changes have been provided.
Expand Down
21 changes: 14 additions & 7 deletions src/logger.ts
Expand Up @@ -12,16 +12,23 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import {Logger, Level} from 'pino';
import * as Pino from 'pino';
import {Logger} from './types';
let logger: Logger;

function setupLogger(userLogger?: Logger | Pino.LoggerOptions) {
if (typeof userLogger === 'undefined' || typeof userLogger === 'object') {
logger = Pino(userLogger);
class NullLogger implements Logger {
error = () => {};
warn = () => {};
info = () => {};
debug = () => {};
trace = () => {};
}

function setupLogger(userLogger?: Logger) {
if (userLogger) {
logger = userLogger;
} else {
logger = userLogger as Logger;
logger = new NullLogger();
}
}

export {logger, Logger, Level, setupLogger};
export {logger, setupLogger};
15 changes: 15 additions & 0 deletions src/types.ts
Expand Up @@ -177,3 +177,18 @@ export interface Hunk {
readonly previousLine?: string;
readonly nextLine?: string;
}

interface LogFn {
/* eslint-disable @typescript-eslint/no-explicit-any */
<T extends object>(obj: T, msg?: string, ...args: any[]): void;
(msg: string, ...args: any[]): void;
/* eslint-enable @typescript-eslint/no-explicit-any */
}

export interface Logger {
error: LogFn;
warn: LogFn;
info: LogFn;
debug: LogFn;
trace: LogFn;
}
3 changes: 1 addition & 2 deletions test/util.ts
Expand Up @@ -15,7 +15,6 @@
import {setupLogger, logger} from '../src/logger';
import {Octokit} from '@octokit/rest';
import {disableNetConnect} from 'nock';
import * as Pino from 'pino';

const octokit = new Octokit();

Expand All @@ -24,7 +23,7 @@ const octokit = new Octokit();
*/
function setup() {
disableNetConnect();
setupLogger(Pino({level: 'warn'}));
setupLogger(console);
}

export {logger, octokit, setup};

0 comments on commit 3e4df30

Please sign in to comment.