Yesterday I had my code reviewed by someone else for the first time. As one of our assignments for the open source development course I'm taking, we had to review each other's code. For this exercise I paired up with Vinh, a friend of mine who also happens to be a pretty good programmer. We were tasked with testing and filing issues on each other's work for the command line tool each we've been working on in the course.
Asynchronous vs Synchronous Approach
We did our code reviews both synchronously over text and asynchronously over GitHub issues. I found the synchronous approach led to faster results as I could consult the author of the code on why they took a certain approach when writing their code, and immediately get a response. However, the asynchronous approach removes the need to find a fixed time in both peoples' schedules to get the work done.
Testing Vinh's program
Vinh created a command-line tool called barrierless that uses AI to translate text phrases into other languages, which I thought was a cool idea. When I began testing Vinh's program, it was still in early development, and so it didn't have a README yet (it does now, go check it out!).
vinhyan
/
barrierless
Barrierless is an open-source command-line tool designed to break down language barriers by providing seamless translations between languages. It allows users to quickly and easily translate text files into their target language, making communication across different languages effortless.
Barrierless
What is Barrierless
Barrierless is a command-line tool designed to break down language barriers by providing seamless translations from one language to another. Powered by GROQCloud, this tool allows users to quickly translate text files into their desired target language, making communication across different languages effortless.
Features
- Auto-detects languages.
- Multiple Language Support: Translate text between a wide range of languages.
- Now supporting GROQCloud and GeminiAI
- Easy to Use: Simple command-line interface for quick translations.
- Customizable: Easily extendable for additional language features or API support.
How to use
Set Up
Please refer to CONTRIBUTING.md
Running the Tool
bl-bot <input files> [-l <target language>]
Note
If no target language specified, it is default to English.
Arguments
-
<input files>: Input filenames need to be translated (required)
Options
-
-v, --version: Barrierless version -
-l, --language <lang>: Target language for translation (default: "English") -
-h, --help: Display…
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/catchblock around anasyncfunction call
Uncaught exception in index.js
#7
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 ${text}...`));
await main(text, options.language);
});
- Some suggestions to make code easier to understand
Could simplify code
#8
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.jsseems unnecessary since it just contains a single function which places arguments into a template literal and returns them - [x] Exporting
main()andgetGroqChatCompletion()seems unnecessary
- Adding a comment to explain the use of both
importandrequirestatements
Add comments explaining mixed import/require
#9
The project uses both ES6 import and CommonJS require due to the chalk module requiring the use of import and using import for package.json leading to an error. It would be helpful to add a comment explaining this.
My turn
Next it was my turn to be reviewed. I wasn't sure what kind of issues would turn up, but Vinh ended up finding a bunch of issues I hadn't paid attention to:
- Adding
npm linkas another option for not having to prefix the tool withnodein the instructions in theREADME
README.md does not include instruction to run `npm link`
#2
The README.md file is missing instructions to run npm link, which is necessary for local development and testing of the CLI tool
- Unnecessary command assignment using
commander.js
- A variable name typo
Conclusion
I thought I'd done a pretty good job but goes to show there's always a bug that might've been missed or a feature that could be improved. It was great having a fresh pair of eyes scrutinize the code I wrote. For now, I fixed the typo and updated the README, but the other issues require testing and I plan on getting to those before I release version 0.1.
Top comments (0)