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

Fix lint:ci problems #16

Merged
merged 8 commits into from
Apr 22, 2024
Merged

Fix lint:ci problems #16

merged 8 commits into from
Apr 22, 2024

Conversation

aneuwald-ctw
Copy link
Collaborator

@aneuwald-ctw aneuwald-ctw commented Apr 15, 2024

User-Facing Changes
After merging different versions to create a fork, we encountered several failures in the CI pipeline's lint checks. This PR addresses these issues by implementing fixes to ensure compliance with our enhanced linting rules in the CI environment. Below are the key changes made:

Description
Once we merged different versions to create the fork, some errors on lint (the lint on CI, that have more checks) started to fail. So basically it was fixed the problems found in the lint ci commands :

  • Yarn Deduplication: The yarn dedupe --check command identified duplicates in yarn.lock. Running yarn dedupe has resolved these duplications.
  • Linting Enhancements: Adjustments were made to resolve linting issues surfaced by the yarn lint:ci command. These issues were not previously detected during development due to the additional lint rules applied only in the CI environment.
  • Unused Exports Cleanup: The yarn lint:unused-exports script helped identify and remove duplicated files and unused export statements resulting from the merge.
  • Dependency Checks: The yarn lint:dependencies script required adding certain libraries to the package.json within packages to meet dependency rules.

These changes ensure that our codebase meets the stricter linting standards of our CI pipeline, leading to cleaner and more maintainable code.

@@ -16,7 +16,7 @@ import { useConfirm } from "@foxglove/studio-base/hooks/useConfirm";

const log = Logger.getLogger(__filename);

export const AVATAR_ICON_SIZE = 42;
const AVATAR_ICON_SIZE = 42;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Out of curiosity: why it was changed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This export was never used. It is shown when using the yarn lint:unused-exports

@@ -91,6 +91,7 @@
"@types/uuid": "9.0.7",
"@types/wicg-file-system-access": "2020.9.6",
"@uiw/react-textarea-code-editor": "3.0.2",
"async-mutex": "0.5.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is this aditional library for?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Required when running yarn lint:dependencies

@laisspportugal
Copy link
Collaborator

Can you please enhance the PR description to make it more clear the reason why those changes needed to be done to fix the pipeline?

@aneuwald-ctw aneuwald-ctw merged commit c092484 into main Apr 22, 2024
6 of 10 checks passed
@aneuwald-ctw aneuwald-ctw deleted the fix-lint-ci-pipeline-errors branch April 22, 2024 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants