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

fix eslint #378

Merged
merged 29 commits into from
Sep 24, 2021
Merged

fix eslint #378

merged 29 commits into from
Sep 24, 2021

Conversation

ljiawliu
Copy link
Contributor

No description provided.

@xqxian xqxian self-requested a review September 18, 2021 08:06
@@ -31,12 +31,12 @@ export default class Commander {
if (Object.prototype.toString.call(name) !== '[object String]') {
return;
}
name = name.toLowerCase();
const invisibleCommand = this.invisibleStore[name];
const newName = name.toLowerCase();
Copy link
Collaborator

Choose a reason for hiding this comment

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

finalName 比较好

if (!optionItemConfig.name) {
optionItemConfig.name = option;
}

optionDescritionItem = optionItemConfig;
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里应该浅拷贝一份

Copy link
Contributor Author

Choose a reason for hiding this comment

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

应该是深拷贝吧?

Copy link
Collaborator

Choose a reason for hiding this comment

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

浅就行

const commandConfig = commandsConfig[command];
registerDevkitCommand(command, commandConfig, directoryPath, ctx);
}
const commandsConfigKey = Object.keys(commandsConfig);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Object.entries

@@ -39,15 +34,16 @@ export default class Hook {
this.emit(EVENT_DONE);
});
break;
default:
default: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

不需要加大括号吧?

const listeners = this.listeners[type];
if (!listeners) {
return;
}
this.listeners[type].forEach((listener: any) => {
listener.apply(null, args);
listener.apply(...args);
Copy link
Collaborator

@xqxian xqxian Sep 18, 2021

Choose a reason for hiding this comment

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

null 不能去除,逻辑改变了;args 也不能展开,第二个参数就是个数组

async _write(data: any, enc: any, callback: any) {
const level = data.level;
const loggerName = data.name || (logger.name && logger.name.split('/').pop()) || PLUGE_NAME;
async newWrite(data: any, enc: any, callback: any) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

不以下划线开头就可以了吧?没必要加个 new

const { hasTimer } = process.env;
let KYE_FILE = {};
let KYE_FILE: any = {};
Copy link
Collaborator

Choose a reason for hiding this comment

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

object 类型比较好

Copy link
Contributor Author

Choose a reason for hiding this comment

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

给object会报错

Copy link
Collaborator

Choose a reason for hiding this comment

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

定义一个 AnyObject

console.log('send success');
}
}
async function report(logObj?: any) {
let readData = fs.readFileSync(LOGGER_LOG_PATH, 'utf-8');
let readData: any = fs.readFileSync(LOGGER_LOG_PATH, 'utf-8');
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里显然是 string 类型

str += prop + ' = ' + config[prop] + '\n';
}
const configKey = Object.keys(config);
configKey.forEach((item: any) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

key 和 value 都需要用时直接用 Object.entries 遍历

const desc = store[name].desc || '';
const arr: any = [];
const storeKey = Object.keys(store);
storeKey.forEach((item: any) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Object.entries

const data = res.data || {};
return data.data && data.data[0];
return (data.data && data.data.length > 0 && data.data[0]) || [];
Copy link
Collaborator

Choose a reason for hiding this comment

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

改变了原逻辑,data.data?.[0] 即可

@@ -64,12 +63,11 @@ async function getRepoInfo(ctx: any, packageName: string) {
// encodeURIComponent("github.com/tencent/feflow.git")
function getGitRepoName(repoUrl: string): string | undefined {
const ret = /^((http:\/\/|https:\/\/)(.*?@)?|git@)/.exec(repoUrl);
let newRepoUrl: String = '';
Copy link
Collaborator

Choose a reason for hiding this comment

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

初始值应为 repoUrl;String 类型应用小写

}
ctx.logger.info(`[${dep}] has installed the latest version: ${hasInstallDep[depName]}`);
return [];
Copy link
Collaborator

Choose a reason for hiding this comment

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

原逻辑没有返回空数组

};

module.exports.installPlugin = installPlugin;
module.exports.updateUniversalPlugin = updateUniversalPlugin;
Copy link
Collaborator

Choose a reason for hiding this comment

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

确定没有模块使用?

// for (const obj in source) {
// target[obj] = source[obj];
// }
const newTarget = JSON.parse(JSON.stringify(target));
Copy link
Collaborator

Choose a reason for hiding this comment

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

浅拷贝即可

// target[obj] = source[obj];
// }
const newTarget = JSON.parse(JSON.stringify(target));
const sourceValue = Object.keys(source);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Object.entries

}
return target;
const extend = function (target: any, source: any) {
// for (const obj in source) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

注释掉的代码直接删除

const pkg = require('../../../package.json');
const { getPkgInfo, updateUniversalPlugin } = require('../native/install');
const version = pkg.version;
// import { getPkgInfo, updateUniversalPlugin } from '../native/install';
Copy link
Collaborator

Choose a reason for hiding this comment

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

删除

interface ErrorInstance {
name: string;
message: string;
stack: string;
}

const pkg = require('../../../package.json');
const { getPkgInfo } = require('../native/install');
const version = pkg.version;
// import { getPkgInfo } from '../native/install';
Copy link
Collaborator

Choose a reason for hiding this comment

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

删除

});
logger.debug('tnpm plugins update infomation', plugins);
if (plugins.length) {
const newPlugins = plugins.filter((plugin: any) => plugin?.name);
Copy link
Collaborator

Choose a reason for hiding this comment

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

叫 pluginsWithName 比较语义化

alexqxxu and others added 8 commits September 23, 2021 11:20
}: {
logger: any;
universalPkg: UniversalPkg;
universalModules: string;
} = ctx;
const serverUrl = ctx.config?.serverUrl;

installPluginStr = installPluginStr.trim();
const newInstallPluginStr = installPluginStr.trim();
Copy link
Collaborator

Choose a reason for hiding this comment

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

finalInstallPluginStr

}
return target;
const extend = function (target: any, source: any) {
const newTarget = target;
Copy link
Collaborator

@xqxian xqxian Sep 23, 2021

Choose a reason for hiding this comment

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

浅拷贝一下否则没有意义

for (const k in oInstalled) {
const version = oInstalled[k];
Object.entries(oInstalled).forEach(([key, value]) => {
const version:any = value;
Copy link
Collaborator

Choose a reason for hiding this comment

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

应为 string 类型

@@ -5,11 +5,11 @@ export function toInstalled(oInstalled: any): Map<string, string> {
if (!oInstalled) {
return installed;
}
for (const k in oInstalled) {
const version = oInstalled[k];
Object.entries(oInstalled).forEach(([key, value]) => {
Copy link
Collaborator

@xqxian xqxian Sep 23, 2021

Choose a reason for hiding this comment

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

key 和 value 应该尽量语义化,例如 value 在最初的代码里已经明确定义为 version 了。其他类似情况不再赘述。

--story=20210826
@@ -9,11 +9,11 @@ function loadModuleList(ctx: any) {
const packagePath = ctx.rootPkg;
const pluginDir = path.join(ctx.root, 'node_modules');
const extend = function (target: any, source: any) {
const newTarget = target;
const targetOpey = { ...target };
Copy link
Collaborator

Choose a reason for hiding this comment

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

opey 是什么意思?叫 newTarget targetCopy 都可以

v_ljiawliu and others added 5 commits September 23, 2021 15:07
return result;
};

export const getGitStatus = (): boolean => {
const command = isWin ? 'where git' : 'which git';
const hasGitCommand = exec(command);
const hasGitDir = fs.existsSync(path.join(cwd, '.git'));
return hasGitCommand && hasGitDir;
return Boolean(hasGitCommand && hasGitDir);
Copy link
Collaborator

Choose a reason for hiding this comment

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

不需要 包多一层 Boolean

@@ -10,7 +10,7 @@ class Report {
ctx: ReportContext;
cmd: string;
args: object;
commandSource: string;
commandSource: string | undefined;
Copy link
Collaborator

Choose a reason for hiding this comment

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

commandSource?: string 即可

@@ -21,58 +21,19 @@ class Report {

constructor(feflowContext: ReportContext, cmd?: string, args?: any) {
this.ctx = feflowContext;
this.cmd = cmd;
this.cmd = cmd!;
Copy link
Collaborator

Choose a reason for hiding this comment

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

cmd 可能 undefined

@@ -28,7 +18,7 @@ function getReportBody(cmd, params): any {
.load('command_source', commandSource)
.load('user_name', userName)
.load('params', params)
.load('err_message', getKeyFormFile(cachePath, 'errMsg'))
.load('err_message', getKeyFormFile(cachePath!, 'errMsg'))
Copy link
Collaborator

Choose a reason for hiding this comment

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

不要强制断言存在,实在不会改先 ts-ignore,我后面统一优化

--story=20210826
@@ -21,58 +21,19 @@ class Report {

constructor(feflowContext: ReportContext, cmd?: string, args?: any) {
this.ctx = feflowContext;
this.cmd = cmd;
this.cmd = cmd || '';
Copy link
Collaborator

Choose a reason for hiding this comment

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

让 cmd?: string

--story=20210826
Copy link
Collaborator

@xqxian xqxian left a comment

Choose a reason for hiding this comment

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

LGTM

@xqxian xqxian merged commit ae60db9 into Tencent:master Sep 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants