Skip to content

Enhance Linting Rules: Add Prettier, Stylelint, and Stricter ESLint Configurations #151

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

Merged
merged 4 commits into from
Jun 26, 2025

Conversation

pplancq
Copy link
Contributor

@pplancq pplancq commented Jun 19, 2025

Description

  • Noticed that the current ESLint rules were quite light, so I propose to strengthen them to ensure better code quality.
  • Added Prettier for consistent code formatting.
  • Integrated Stylelint for CSS/SCSS linting.
  • For efficiency, I used my own shared configurations (@pplancq), but if you prefer, I can manually add all the configuration files instead.

I was also considering setting up Husky, a tool that allows you to add Git hooks in order to enforce code linting at commit time. I would like to know your opinion on this subject to see if I can integrate it into the project.

Related issue(s)

N/A

@SebastienDegodez
Copy link
Member

@lbroudoux the usage of the pplancq packages has been well received, based on the feedback I’ve received. This one offers the flexibility to customize as needed without any issues. The overall rules are appreciated, and even in the event of a shift in direction, they can still be adapted or reused easily (copy).

@SebastienDegodez
Copy link
Member

About Husky, I use in dotnet implementation (dotnet husky) but it's the same.

@lbroudoux
Copy link
Member

I like consistency and helping maintainability so it's a +1 here!

I have one additional feature request, though: could we also ensure that files containing code (TS, TSX or JS here) contain the regular copyright header?

The model we have for Java files is:

/*
 * Copyright The Microcks Authors.
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *  http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

@lbroudoux lbroudoux added this to the 0.9.0 milestone Jun 20, 2025
@pplancq
Copy link
Contributor Author

pplancq commented Jun 20, 2025

Thanks for your feedback!
Yes, I did notice in a previous PR that you intended to include this header consistently across all code files.
That actually ties in with an idea I had: setting up a Git hook (using Husky, for instance) to automatically add this header to newly committed files. I’d be happy to submit a dedicated PR for that. The PR could also provide feedback on existing files in the React project to help ensure consistency.

@pplancq
Copy link
Contributor Author

pplancq commented Jun 20, 2025

I've set up Husky to introduce Git hooks for better control during development:

  • Commit message validation using commitlint
  • Pre-commit checks with lint-staged, running ESLint, Stylelint, Prettier, and TypeScript on staged files

To make this work properly, I updated the package.json to a workspace configuration, allowing us to share dependencies across packages. For now, I've only included the React project in the workspace. I preferred not to modify the old frontend or server dependencies just yet.

Eventually, the server will also be integrated into the workspace.

You might also notice a volta key in the package.json files. This has no impact on the project—Volta is a Node version manager I use to easily switch Node versions between projects. I pinned Node 18 for the server, since I saw that the Dockerfile uses a nodejs-18-minimal image.

Lastly, I updated the installation instructions for the new frontend in the INSTALLATION.NEXT.md file.

I hope these changes are helpful and align with what you were expecting!

Copy link
Member

@lbroudoux lbroudoux left a comment

Choose a reason for hiding this comment

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

This is globally OK for me but I'm concerned about integrating dependencies on @pplancq packages: not about the quality of your work but about license compatibility issues and real need of non main stream deps.

Could you provide more explanations on these whilst I check if the license is actually an issue?

pplancq added 4 commits June 26, 2025 10:22
…atting

Signed-off-by: Paul PLANCQ <paul.plancq@outlook.fr>
Signed-off-by: Paul PLANCQ <paul.plancq@outlook.fr>
Signed-off-by: Paul PLANCQ <paul.plancq@outlook.fr>
…tend

Signed-off-by: Paul PLANCQ <paul.plancq@outlook.fr>
Copy link
Member

@lbroudoux lbroudoux left a comment

Choose a reason for hiding this comment

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

LGTM

@lbroudoux lbroudoux merged commit 58fa040 into microcks:master Jun 26, 2025
5 checks passed
@pplancq pplancq deleted the feature/linter branch June 26, 2025 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants