website icon indicating copy to clipboard operation
website copied to clipboard

ER: Adopting a coding style standard/guideline

Open tony1ee opened this issue 9 months ago • 4 comments

Dependency

  • #4349

Emergent Requirement - Problem

As our project grow and attract contributions from a diverse group of developers, we have different styles of coding styles across our codebase (trailing spaces, spaces between operators, braces style, etc.). This poses a challenge for readability, maintainability and collaboration.

Proposing we explore the feasibility of adopting a unified coding format standard.

Issue you discovered this emergent requirement in

  • #6777

Date discovered

05/01/2024

Did you have to do something temporarily

  • [ ] YES
  • [x] NO

Who was involved

@tony1ee @gaylem

What happens if this is not addressed

  • Different styles of code pose a challenge for readability, maintainability and collaboration.
  • The absence of guidelines for naming, formatting, and commenting practices could adversely affect our professionalism and compromise the quality of our work.

Resources

Why Prettier Firefox CSS Guidelines VS Code Formatter: TypeScript Airbnb JavaScript Style Guide Google JavaScript Style Guide

Recommended Action Items

  • [ ] Make a new issue
  • [ ] Discuss with team
  • [ ] Let a Team Lead know

Potential solutions [draft]

We can solve this in several steps:

  • [ ] Perform an analysis of existing codebase, including the language and file type.
  • [ ] Research existing standardized guidelines and practices (potentially for each file type), then choose/define our standard
  • [ ] Creating documents for reference and training
  • [ ] Explore tooling and automation solutions: linters and formatters configurations in code editors, GHA checkers, etc.
  • [ ] Consider either incremental refactoring or major refactoring for existing non-conforming code, depending on priority.

tony1ee avatar May 02 '24 08:05 tony1ee

  • @tony1ee @gaylem thank you for presenting these ideas. Here at HfLA I am not familiar with a JS style guide such as the Airbnb or Google JS Style Guides, but I really liked the AIrbnb JS Style Guide and how it referenced the ESLint rules, so if a rule came up in VS Code, a dev could search the guide and find relevant advice.

@ExperimentsInHonesty are you interested in making use of a JS style guide in some manner?

roslynwythe avatar May 06 '24 07:05 roslynwythe

Regarding linters - I want to mention that there was some work done on ESLint and eslint:prettier

  • #5023

  • #5024

  • #5258

  • #3230

  • #2651

  • #4264

  • You'll note that some of the above are still draft PRs. I believe there are several reasons why the work did not progress:

    • At that time we were still beginning the rollout of the spell checker and CodeQL and didn't want to roll out additional linters until those rollouts were complete
    • liquid./yaml code in JS files caused false positives with ESLint. Solutions discussed were:
      • moving liquid code to HTML - there are some samples of this in #5258
      • ESLint recognizes comments such as // eslint-disable-next-line which ignores the rules for the next line of code, or // eslint-disable-line, which ignores the rules for the current line
    • For use on GitHub PRs, the GHA Super-linter was used. While Super-linter supports CSS and ESLint, it did not have a spell checker that was compatible with the Code Spell Checker for VS Code. So we might want to consider Mega-linter.

@ExperimentsInHonesty are you interested in restarting the discussion around ESLint and prettier?

roslynwythe avatar May 06 '24 07:05 roslynwythe

@roslynwythe @tony1ee

@ExperimentsInHonesty are you interested in restarting the discussion around ESLint and prettier?

After all the spelling issues are addressed.

ExperimentsInHonesty avatar May 06 '24 22:05 ExperimentsInHonesty

For future discussion: Since we are talking about linters, here are comments regarding the lint-scss.yml workflow and the github/super-linter

t-will-gillis avatar May 11 '24 21:05 t-will-gillis