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

Set up ESLint for post service #3216

Closed
wants to merge 3 commits into from

Conversation

aserputov
Copy link
Contributor

Issue This PR Addresses

Fixes #3157

Type of Change

  • Bugfix: Change which fixes an issue
  • New Feature: Change which adds functionality
  • Documentation Update: Change which improves documentation
  • UI: Change which improves UI

Description

Steps to test the PR

Checklist

  • Quality: This PR builds and passes our npm test and works locally
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not (if applicable)
  • Documentation: This PR includes updated/added documentation to user exposed functionality or configuration variables are added/changed or an explanation of why it does not(if applicable)

@aserputov aserputov added the dependencies Pull requests that update a dependency file label Mar 15, 2022
@aserputov aserputov added this to the 2.9 Release milestone Mar 15, 2022
@aserputov aserputov requested a review from cindyorangis March 15, 2022 16:14
@aserputov aserputov self-assigned this Mar 15, 2022
@gitpod-io
Copy link

gitpod-io bot commented Mar 15, 2022

src/api/posts/package.json Outdated Show resolved Hide resolved
TueeNguyen
TueeNguyen previously approved these changes Mar 15, 2022
TueeNguyen
TueeNguyen previously approved these changes Mar 15, 2022
cindyorangis
cindyorangis previously approved these changes Mar 15, 2022
@aserputov aserputov requested review from joelazwar and RC-Lee March 16, 2022 13:40
RC-Lee
RC-Lee previously approved these changes Mar 16, 2022
@RC-Lee
Copy link
Contributor

RC-Lee commented Mar 16, 2022

Make sure to rebase and resolve conflicts first please. Thanks!

@aserputov
Copy link
Contributor Author

aserputov commented Mar 17, 2022

@cindyledev while working on this issue, I discovered that there is a bug if we use @senecacdot/eslint-config-telescope": "1.0.0". At first, I thought that problem was with rules in .eslint. The bug:

[Definition for rule '@typescript-eslint/no-shadow' was not found](https://stackoverflow.com/questions/66033302/definition-for-rule-typescript-eslint-no-shadow-was-not-found 

To fix it, I changed all of the dependencies to 1.0.0. This bug appeared after I rebased.

@cindyorangis
Copy link
Contributor

According to #3247, we have chosen to unlink local packages so any eslint-config-telescope shouldn't be using workspace:1.0.0. It should be 1.0.0 instead

@aserputov
Copy link
Contributor Author

@cindyledev 1:1 error Definition for rule '@typescript-eslint/no-shadow' was not found @typescript-eslint/no-shadow. Do you have any guess, why this can be? I think it's not only on my side.

@@ -25,6 +28,8 @@
"normalize-url": "6.1.0"
},
"devDependencies": {
"@senecacdot/eslint-config-telescope": "1.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

We just updated eslint-config-telescope to v1.1.0 in #3267 and many of the eslint configurations were refactored in #3253. To pick up the latest updates which addresses the no-shadow error, change the 1.0.0 to 1.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Configure ESLint for posts
4 participants