Revive and improve old project #61
Conversation
Major updates: - Updated commander from 2.20.0 to 12.1.0 - Replaced cli-table with cli-table3 (more maintained) - Replaced osmosis with axios + cheerio (Node.js 22 compatible) - Removed clear dependency (using native console.clear()) - Added eslint 9.16.0 with modern flat config - Migrated to async/await pattern - Updated dependabot configuration with grouped updates Breaking changes: - Requires Node.js >= 18.0.0 - Web scraping may be blocked by bab.la's anti-bot protection See UPGRADE_NOTES.md for detailed information.
Summary of ChangesHello @tcelestino, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request undertakes a significant overhaul to modernize an older project, focusing on updating outdated dependencies, improving code quality, and enhancing maintainability. It addresses compatibility issues with newer Node.js versions, streamlines the web scraping mechanism, and introduces a robust linting setup. While these changes bring the project up to current standards, a known challenge with anti-bot protection on the target website has been identified and documented for future resolution. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a significant and well-executed modernization of the project. The dependency updates, migration to async/await, and adoption of a modern ESLint configuration are all excellent improvements. I've provided a few suggestions to further enhance error handling, improve the readability of the output, and make the ESLint configuration more robust by including the recommended ruleset.
| module.exports = [ | ||
| { | ||
| languageOptions: { | ||
| ecmaVersion: 2021, | ||
| sourceType: 'commonjs', | ||
| globals: { | ||
| console: 'readonly', | ||
| process: 'readonly', | ||
| require: 'readonly', | ||
| module: 'readonly', | ||
| __dirname: 'readonly', | ||
| __filename: 'readonly' | ||
| } | ||
| }, | ||
| rules: { | ||
| 'indent': ['error', 2], | ||
| 'no-console': 0, | ||
| 'linebreak-style': ['error', 'unix'], | ||
| 'quotes': ['error', 'single'], | ||
| 'semi': ['error', 'always'] | ||
| } | ||
| } | ||
| ]; |
There was a problem hiding this comment.
Your new ESLint configuration is missing eslint:recommended, which was present in the old .eslintrc file. This means you are losing many valuable core linting rules. Also, manually defining all Node.js globals is verbose and can be error-prone.
It's better to extend the recommended configuration and use the globals package for defining the environment. You'll need to add globals to your devDependencies (npm install -D globals).
const js = require('@eslint/js');
const globals = require('globals');
module.exports = [
js.configs.recommended,
{
languageOptions: {
ecmaVersion: 2021,
sourceType: 'commonjs',
globals: {
...globals.node,
},
},
rules: {
'indent': ['error', 2],
'no-console': 'off',
'linebreak-style': ['error', 'unix'],
'quotes': ['error', 'single'],
'semi': ['error', 'always'],
},
},
];
conjugate.js
Outdated
| }); | ||
|
|
||
| if (type && overview.length > 0) { | ||
| results.push({ type, overview: overview.join(', ') }); |
There was a problem hiding this comment.
When a verb tense has multiple conjugation forms (e.g., 'I am', 'you are', 'he/she/it is' for the present tense of 'to be'), joining them with a comma might make the table output wide and hard to read. Consider joining them with a newline character ('\n') to display each form on a new line within the same table cell, improving readability.
| results.push({ type, overview: overview.join(', ') }); | |
| results.push({ type, overview: overview.join('\n') }); |
conjugate.js
Outdated
| } catch (err) { | ||
| console.error(`Error: The verb "${verb}" was not found or there was a connection issue`); | ||
| console.error(err.message); | ||
| } |
There was a problem hiding this comment.
The error handling can be made more specific. By inspecting the error object from axios, you can provide more informative messages to the user, for example, distinguishing between a 404 'Not Found' error and other connection issues or anti-bot protections (like a 403 'Forbidden' error, as mentioned in UPGRADE_NOTES.md).
} catch (err) {
if (err.response && err.response.status === 404) {
console.error(`Error: The verb "${verb}" was not found.`);
} else {
console.error(`Error: Failed to retrieve conjugation for "${verb}". This could be a connection issue or the site may be blocking requests.`);
if (err.message) {
console.error(err.message);
}
}
}- Install Vitest 4.0.7 with coverage and UI support - Refactor code for testability by extracting functions to lib/conjugate.js - Create 18 unit tests covering: * HTML parsing (parseConjugationResults) * Input validation (searchVerb) * Error handling (network errors, no results) * Result formatting (formatResults) * Console display (displayResults) - Achieve 84% code coverage - Configure Vitest with v8 coverage provider - Add test scripts: test, test:watch, test:ui, test:coverage All tests passing: 18 passed, 1 skipped (integration test)
Add comprehensive CI/CD setup with: GitHub Actions Workflows: - ci.yml: Run tests on all PRs and pushes to main/master * Test matrix: Node.js 18.x, 20.x, 22.x * Run linter on all versions * Generate code coverage reports * Upload coverage to Codecov (optional) * Verify CLI installation - release.yml: Automated releases on version tags * Generate changelog from git history * Create GitHub Release with notes * Publish to npm (optional, requires NPM_TOKEN) Documentation: - CI_CD_SETUP.md: Complete CI/CD configuration guide - RELEASING.md: Step-by-step release process - PULL_REQUEST_TEMPLATE.md: PR template with checklist README Updates: - Add status badges (CI, npm, license, Node.js version) - Add Development section with test commands - Update Contributing section with test requirements - Add Roadmap with completed and future features - Remove outdated To Do section This ensures all pull requests are automatically tested before merge, maintaining code quality and preventing regressions.
Major updates:
Breaking changes:
See UPGRADE_NOTES.md for detailed information.