Skip to content
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

[Linting] Implement lint-all #6968

Closed
unlikelyzero opened this issue Aug 19, 2023 · 2 comments · Fixed by #6969
Closed

[Linting] Implement lint-all #6968

unlikelyzero opened this issue Aug 19, 2023 · 2 comments · Fixed by #6969
Labels
help_wanted Help the Open MCT project! type:maintenance tests, chores, or project maintenance
Milestone

Comments

@unlikelyzero
Copy link
Collaborator

Proposal: Improve Linting Workflow

Summary

We're introducing additional linting rules to enhance the consistency and clarity of our commits.

To maintain the efficiency of our CI runtime, it's advantageous to run all linting stages concurrently within a single step.


Parallel Execution with concurrently or run-all:

Pros:

  • Faster Execution: All linting processes run at the same time, potentially reducing the total linting time.
  • Efficiency: Ideal for CI environments where multiple cores are available, as it can utilize resources better.

Cons:

  • Additional Dependency: Incorporating third-party libraries introduces another dependency which might impact our build/test/install process.
  • Muddled Output: The output from simultaneous linting processes can become intermixed, making it harder to interpret and resolve issues.

Sequential Execution:

Pros:

  • Clear Output: Since tasks run one after the other, the output is easier to interpret. It's straightforward to pinpoint issues without logs getting mixed.
  • Simplicity: No additional dependencies or configurations are needed.

Cons:

  • Slower Execution: Especially during local development, running linting tasks one by one can be slower.

Given the trade-offs, and considering our CI's feedback loop is roughly a minute, I'm leaning towards option 2 (Sequential Execution). It seems a more suitable choice for our needs, even if it's slightly slower during local development.

Proposed Change

Here's the proposed update to our scripts in the package.json:

{
  "scripts": {
    "lint": "eslint example src e2e --ext .js openmct.js --max-warnings=0 && eslint example src --ext .vue",
    "lint:spelling": "cspell \"**/*.{js,md,vue}\" --show-context --gitignore",
    "fix:lint": "eslint example src e2e --ext .js,.vue openmct.js --fix",
    "lint:all": "run-p lint:*"
  }
}
@unlikelyzero unlikelyzero added the type:maintenance tests, chores, or project maintenance label Aug 19, 2023
@unlikelyzero
Copy link
Collaborator Author

@evenstensberg please take a look at this. I think we should begin a path towards running many linting rules before we get too far ahead of ourselves

@unlikelyzero unlikelyzero added this to the Target:3.1.0 milestone Aug 19, 2023
@unlikelyzero unlikelyzero added the help_wanted Help the Open MCT project! label Aug 19, 2023
@evenstensberg
Copy link
Contributor

@unlikelyzero submitted a PR :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help_wanted Help the Open MCT project! type:maintenance tests, chores, or project maintenance
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants