Added ESLint + Prettier and GitHub Actions lint workflow#96
Added ESLint + Prettier and GitHub Actions lint workflow#96Mehulantony wants to merge 6 commits intooss-slu:mainfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces automated code quality enforcement by adding ESLint, Prettier, and a GitHub Actions lint workflow to the OSS Rerum Playground project. The changes standardize code formatting across JavaScript files and establish automated checks for future contributions.
Key Changes:
- Added ESLint v9.39.1 with Prettier integration for code linting and formatting
- Created GitHub Actions workflow to run lint and format checks on pushes and PRs
- Applied automated formatting fixes to multiple JavaScript files (style-only changes)
Reviewed Changes
Copilot reviewed 13 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
package.json |
Added ESLint, Prettier, and related plugins as dev dependencies with npm scripts for linting and formatting |
package-lock.json |
Lockfile for npm dependencies |
eslint.config.js |
ESLint flat config with Prettier integration and browser globals |
.prettierrc.json |
Prettier formatting rules (100 char width, 2 space tabs, single quotes) |
.prettierignore |
Excludes node_modules, dist, vendor, and minified files from formatting |
.gitignore |
Added node_modules/ to version control exclusions |
.github/workflows/lint.yml |
CI workflow to run ESLint and Prettier checks on PRs and main branch pushes |
web/js/utilities.js |
Applied automated formatting (indentation, spacing, semicolons, quote style) |
web/js/toolsCatalog.js |
Applied automated formatting |
web/js/tools.js |
Applied automated formatting; contains duplicate code that needs removal |
web/js/sandbox.js |
Applied automated formatting |
web/js/playground.js |
Applied automated formatting |
web/js/manifestStorage.js |
Applied automated formatting |
web/js/json-utils.test.js |
Applied automated formatting |
web/js/json-utils.js |
Applied automated formatting |
web/js/config.js |
Applied automated formatting |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| window.updateToolOrder = function (toolLabel) { | ||
| const clickedTool = ToolsCatalog.find((tool) => tool.label === toolLabel); | ||
| if (clickedTool) { | ||
| updateRecentlyUsedTools(clickedTool); | ||
| renderTools(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
The window.updateToolOrder function is defined twice in this file (lines 194-200 and lines 300-306). This creates duplicate code that should be removed. Keep only one definition of this function.
| document.addEventListener('DOMContentLoaded', () => { | ||
| /** | ||
| * These are promises so we can control the chaining how we like, if necessary. | ||
| */ | ||
| try { | ||
| initializeInterfaces(PLAYGROUND.INTERFACES) | ||
| initializeTechnologies(PLAYGROUND.TECHNOLOGIES) | ||
| renderTools(); | ||
| renderStoredManifests(); | ||
| } catch (err) { | ||
| console.error("Error initializing the playground: ", err); | ||
| } | ||
| }); No newline at end of file | ||
| /** | ||
| * These are promises so we can control the chaining how we like, if necessary. | ||
| */ | ||
| try { | ||
| initializeInterfaces(PLAYGROUND.INTERFACES); | ||
| initializeTechnologies(PLAYGROUND.TECHNOLOGIES); | ||
| renderTools(); | ||
| renderStoredManifests(); | ||
| } catch (err) { | ||
| console.error('Error initializing the playground: ', err); | ||
| } | ||
| }); |
There was a problem hiding this comment.
The DOMContentLoaded event listener is registered twice (lines 202-214 and lines 308-320). This will cause the initialization code to run twice, which is unnecessary and could lead to unexpected behavior. Remove one of these duplicate listeners.
| curly: ["warn", "all"], | ||
| "prettier/prettier": "warn" | ||
| }, | ||
| ignores: ["node_modules/", "web/dist/", "web/vendor/", "web/**/*.min.js", "web/**/*.test.js"] |
There was a problem hiding this comment.
The ignores property is placed inside the configuration object, but in ESLint's flat config format, ignores should be a top-level property in a separate configuration object. This should be:
module.exports = [
{
ignores: ["node_modules/", "web/dist/", "web/vendor/", "web/**/*.min.js", "web/**/*.test.js"]
},
{
files: ["web/**/*.js"],
// ... rest of config
}
];There was a problem hiding this comment.
This is a good suggestion. ESLint supports "ignores" inside the same config object in Flat Config as well as in a separate top-level block. Since the current placement of code works and keeps the PR minimal, I will not be making this change now. We can revisit this later if needed for clarity.
|
The CI workflow was failing earlier (Prettier checks failing) because HTML, CSS and JSON files had formatting differences. Auto-formatting these files would involve huge changes and could potentially introduce visual/UI regressions. In order to avoid this, a modification was made to "package.json" file to only check JavaScript files which aligns with the linting rules already implemented in this project and ensures consistent style enforcement without touching other unrelated assets. These adjustments allowed the GitHub Actions CI pipeline to complete with a green check, confirming that the linting and formatting checks pass reliably for all JS files moving forward. Some ESLint warnings (mainly related to unused variables in legacy functions) still appear during the linting process, and is visible in the GitHub Actions workflow. These warnings do not impact the functionality of the application, and addressing them would require modifying production JavaScript logic, which is outside the scope of this CI/CD-focused PR. To avoid unintentionally changing behavior in a user-facing codebase, these warnings were left untouched. The CI workflow is configured to report these warnings without failing, allowing maintainers to be aware of them while ensuring the pipeline remains stable. |
|
Included changes to Rerum's .gitignore file to exclude common temporary, system, dependency and IDE-specific files which should not be tracked in version control. This update helps ensure that the project repository is clean and prevents accidentally committing large files, thereby improving developer experience. No production code is affected by this change. |
This PR introduces automated code quality enforcement to the OSS Rerum Playground project to improve code maintainability, consistency and contributor experience across the codebase.
What was added:
Why this is important?
Previously, the project did not have automated code quality checks, which could allow inconsistent formatting, undefined variables, or accidental errors into main.
With a lint workflow running on every push and pull request, contributors get immediate feedback and poorly formatted/unsafe code is prevented from merging. It also makes the codebase easier to debug, maintain and extend.
Impact on existing codebase:
3.The application behavior remains unchanged.
How to run locally:
*CI automatically runs these checks on PRs and pushes