最初のコードレビュー

昨日、私は初めて自分のコードを他の人にレビューしてもらいました。私が受講しているオープンソース開発コースの課題の 1 つとして、お互いのコードをレビューする必要がありました。この演習では、友人であり、非常に優れたプログラマーでもある Vinh とペアを組みました。私たちは、コースでそれぞれ取り組んできたコマンド ライン ツールについて、お互いの作業をテストし、問題を提出するという任務を負っていました。

ヴィンニャンフォロー

/ヴィンヒャン

非同期アプローチと同期アプローチ

私たちはコード レビューをテキスト上で同期的に、および GitHub の問題上で非同期的に行いました。コードを作成する際に特定のアプローチを採用した理由についてコードの作成者に相談でき、すぐに返答が得られるため、同期アプローチの方が結果が早くなることがわかりました。ただし、非同期アプローチを使用すると、作業を完了するために両方の人のスケジュールから固定時間を見つける必要がなくなります。

Vinh のプログラムをテストする

Vinh は、AI を使用してテキスト フレーズを他の言語に翻訳する、barrierless と呼ばれるコマンド ライン ツールを作成しました。これは素晴らしいアイデアだと思いました。私が Vinh のプログラムのテストを開始したとき、それはまだ開発初期段階だったので、まだ README がありませんでした (現在はあります。ぜひチェックしてください!)。

git clone git@github.com:vinhyan/barrierless.git

cd barrierless

npm install

A feature I really liked is the colorful output text which makes the user experience a little bit more pleasant - something I neglected in my own program in trying to model it after CLI tools like git.

I read the package.json file to find out how the program should be run, and when it immediately crashed I realized I forgot to add the API key as an environment variable. After adding my API key, the program ran without errors, although I did find an interesting quirk - the program defaults the output language to English, so if you didn't specify one, and the input was in English, it seemed to choose a language to translate to on its own - either randomly, or based on context from the input.

I opened a few other issues, mostly to do with improving code quality:

• A missing try/catch block around an async function call

Uncaught exception in index.js #7

uday-rana posted on Sep 12, 2024

index.js contains the following async function calls which are not wrapped in a try/catch block and may lead to an uncaught exception:

export async function main(text, targetLang) {
  const chatCompletion = await getGroqChatCompletion(text, targetLang);
  console.log(chatCompletion.choices[0]?.message?.content || '');
}

...

program
  ...
  .action(async (text, options) => {
    console.log(chalk.blue(`Translating  <span class="pl-s1"><span class="pl-kos">${text}</span>...`</span>));

    await main(text, options.language);
  });

ログイン後にコピー

View on GitHub

• Some suggestions to make code easier to understand

Could simplify code #8

uday-rana posted on Sep 12, 2024

Some changes may be made to to the project make it easier to understand and work on:

• [x] Move Groq configuration above program initialization with commander
• [x] main() seems unnecessary since it contains two lines of code and there are more lines of code involved in creating and invoking the function than if it was omitted
• [ ] prompt.js seems unnecessary since it just contains a single function which places arguments into a template literal and returns them
• [x] Exporting main() and getGroqChatCompletion() seems unnecessary

View on GitHub

• Adding a comment to explain the use of both import and require statements

Add comments explaining mixed import/require #9

uday-rana posted on Sep 12, 2024

プロジェクトは ES6 インポートと CommonJS の両方を使用しています。これは、Chalk モジュールがインポートの使用を必要とし、package.json にインポートを使用するとエラーが発生するためです。これについて説明するコメントを追加していただけると助かります。

GitHub で表示

私の番

次に私がレビューされる番でした。どのような問題が発生するかわかりませんでしたが、Vinh は最終的に、私が注目していなかった多くの問題を見つけてくれました。

• README の手順でツールの前にノードを付ける必要がないようにするための別のオプションとして npm リンクを追加します。

README.md には「npm link」を実行するための命令が含まれていません。 #2

ヴィンヒャン 投稿日: 2024 年 9 月 12 日

README.md ファイルには、CLI ツールのローカル開発とテストに必要な npm リンクを実行するための手順がありません

GitHub で表示

• commandor.jsを使用した不要なコマンドの割り当て

CLI にはサブコマンドがないため、`program.command("run")` は不要です #3

ヴィンヒャン 投稿日: 2024 年 9 月 12 日

GitHub で表示

• 変数名のタイプミス

変数名のタイプミス #4

ヴィンヒャン 投稿日: 2024 年 9 月 12 日

index.js 31 行目: 変数名のタイプミス: reponseStream

GitHub で表示

結論

私はかなり良い仕事をしたと思っていましたが、見落とされた可能性のあるバグや改善できる機能が常に存在することを示しています。自分が書いたコードを新鮮な目で精査してもらえるのはとてもうれしかったです。現時点では、タイプミスを修正し、README を更新しましたが、他の問題についてはテストが必要なので、バージョン 0.1 をリリースする前に解決する予定です。

以上が最初のコードレビューの詳細内容です。詳細については、PHP 中国語 Web サイトの他の関連記事を参照してください。