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.

Sign up for GitHub

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

Jump to bottom

Fix ESLint errors for core packages #10775

Closed
praveenkuttappan opened this issue Aug 23, 2020 · 3 comments · Fixed by #13724
Closed

Fix ESLint errors for core packages #10775

praveenkuttappan opened this issue Aug 23, 2020 · 3 comments · Fixed by #13724
Assignees
@deyaaeldeen
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library. help wanted This issue is tracking work for which community contributions would be welcomed and appreciated

Comments

@praveenkuttappan
Copy link
Member

praveenkuttappan commented Aug 23, 2020 •
edited
Loading

Fix lint errors found in core packages by ESLint. Following are the steps to run ESLint for core packages and reproduce this issue.

  1. Set up your dev environment if not already done so as mentioned here
  2. Go to <repo root>/sdk/core/<package-name>
  3. run rushx lint
  4. Command in step 2 generates an html report in directory <repo root>/sdk/core/<package-name> with name ends with lintReport.html
  5. All lint errors found in this package is listed on html report.

Once all known issues are resolved, below change is required in package.json file in package root <repo root>/sdk/core/<package-name> to treat any new lint regression as hard failure in CI.

  • Remove following snippet from lint command in package.json
    -f html -o template-lintReport.html || exit 0

Note: HTML report name prefix may be different for each package name to differentiate the report for each package.
@ghost ghost added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Aug 23, 2020
@ramya-rao-a ramya-rao-a added this to the Backlog milestone Aug 25, 2020
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Aug 25, 2020
@ramya-rao-a ramya-rao-a added Azure.Core Client This issue points to a problem in the data-plane of the library. labels Aug 25, 2020
@ramya-rao-a ramya-rao-a added help wanted This issue is tracking work for which community contributions would be welcomed and appreciated and removed Up for grabs labels Sep 15, 2020
This was referenced Sep 28, 2020
Closed
Closed
@ramya-rao-a ramya-rao-a modified the milestones: Backlog, MQ-2020 Nov 16, 2020
@ramya-rao-a ramya-rao-a modified the milestones: MQ-2020, Backlog Dec 17, 2020
@ramya-rao-a ramya-rao-a mentioned this issue Feb 6, 2021
Merged
ghost pushed a commit that referenced this issue Feb 8, 2021
@ramya-rao-a
Fix linting errors in new core pkgs (#13643)
5e06af5 
This PR fixes some of the easy to fix linting errors in the new core packages

Related to #12947 and #10775
This was referenced Feb 9, 2021
Closed
Merged
@ramya-rao-a
Copy link
Contributor

Update: At this moment, only core-amqp and core-xml packages in the sdk/core folder have pending linting errors

cc @xirzec, @chradek

@ramya-rao-a ramya-rao-a mentioned this issue Feb 9, 2021
Merged
@xirzec
Copy link
Member

xirzec commented Feb 9, 2021

core-xml is addressed by #13701

@chradek chradek mentioned this issue Feb 10, 2021
Merged
@ghost ghost closed this as completed in #13724 Feb 17, 2021
ghost pushed a commit that referenced this issue Feb 17, 2021
@chradek
[core-amqp] fix remaining eslint errors (#13724)
43d9a4c 
Fixes #10775 

Brings existing 19 issues down to 0 and makes lint errors fail CI.

Notable: I had to pull the websockets test out into separate browser and node.js files. This was because previously, we were using `require` to conditionally pull in the `ws` dependency for node.js, but the linter wants us to stop usage of `require`. There are dynamic `import` statements but not with es6 modules (it requires es2020 I believe). Separating the test by runtime let me use ES2015 imports to import `ws` just in node.js.
@ramya-rao-a
Copy link
Contributor

Woohoo! Thanks @chradek for taking us over the finish line!

@xirzec xirzec removed this from the Backlog milestone May 17, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@deyaaeldeen deyaaeldeen

Labels
Azure.Core Client This issue points to a problem in the data-plane of the library. help wanted This issue is tracking work for which community contributions would be welcomed and appreciated
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[core-amqp] fix remaining eslint errors chradek/azure-sdk-for-js
4 participants
@xirzec @deyaaeldeen @ramya-rao-a @praveenkuttappan