[Linter] Update files with respect to `sort-imports` rule #18598

bzhang0 · 2021-11-09T09:45:45Z

This solves #9252. This PR is an update from a past PR, #18567.

What's new?

I've added the sort-imports rule into ./common/tools/eslint-plugin-azure-sdk/.eslintrc.json
I've updated files appropriately after calling rush rebuild and rush lint.

I used the default settings for sort-imports, so if there should be changes to the rule settings, please let me know!

ghost · 2021-11-09T09:45:51Z

Thank you for your contribution bzhang0! We will review the pull request and get back to you soon.

deyaaeldeen · 2021-11-09T19:05:23Z

Hi @bzhang0,

Thanks a lot for these changes!

While it is good to do this change and sort imports inside the plugin source code, it does not solve #9252 yet. To do so, the PR will need to add this new behavior as part of the plugin rules list. Once this is done, all imports in all packages across the repository can be sorted to satisfy the rule. Please use rush lint to run the linter for all packages.

bzhang0 · 2021-11-09T20:26:31Z

Oh, I see! I assumed putting the rule in the eslint's .eslintrc.json file was enough.

I will have these changes fixed. Thanks for letting me know!

bzhang0 · 2021-11-09T21:50:56Z

I'm getting an error when trying to call rush rebuild after calling rush update.

The error occurs on @azure/keyvault-certificates:

--[ FAILURE: @azure/keyvault-certificates ]---------[ 1 minute 36.3 seconds ]--

Invoking: npm run clean && tsc -p . && npm run build:nodebrowser && api-extractor run --local 


> @azure/[email protected] clean
> rimraf dist-esm dist-test types *.tgz *.log samples/typescript/dist

error TS6053: File '/Users/ben/Documents/repos/azure-sdk-for-js/sdk/keyvault/keyvault-common/types/challengeBasedAuthenticationPolicy.d.ts' not found.
  The file is in the program because:
    Matched by include pattern '../keyvault-common/**/*.ts' in 'tsconfig.json'
error TS6053: File '/Users/ben/Documents/repos/azure-sdk-for-js/sdk/keyvault/keyvault-common/types/index.d.ts' not found.
  ...1 line omitted...
    Matched by include pattern '../keyvault-common/**/*.ts' in 'tsconfig.json'
error TS6053: File '/Users/ben/Documents/repos/azure-sdk-for-js/sdk/keyvault/keyvault-common/types/parseKeyvaultIdentifier.d.ts' not found.
  The file is in the program because:
    Matched by include pattern '../keyvault-common/**/*.ts' in 'tsconfig.json'
error TS6053: File '/Users/ben/Documents/repos/azure-sdk-for-js/sdk/keyvault/keyvault-common/types/parseWWWAuthenticate.d.ts' not found.
  The file is in the program because:
    Matched by include pattern '../keyvault-common/**/*.ts' in 'tsconfig.json'
error TS6053: File '/Users/ben/Documents/repos/azure-sdk-for-js/sdk/keyvault/keyvault-common/types/tracingHelpers.d.ts' not found.
  The file is in the program because:
    Matched by include pattern '../keyvault-common/**/*.ts' in 'tsconfig.json'

These files all exist within my subdirectory. Any idea what the issue could be?

check-enforcer · 2021-11-10T09:03:01Z

This pull request is protected by Check Enforcer.

What is Check Enforcer?

Check Enforcer helps ensure all pull requests are covered by at least one check-run (typically an Azure Pipeline). When all check-runs associated with this pull request pass then Check Enforcer itself will pass.

Why am I getting this message?

You are getting this message because Check Enforcer did not detect any check-runs being associated with this pull request within five minutes. This may indicate that your pull request is not covered by any pipelines and so Check Enforcer is correctly blocking the pull request being merged.

What should I do now?

If the check-enforcer check-run is not passing and all other check-runs associated with this PR are passing (excluding license-cla) then you could try telling Check Enforcer to evaluate your pull request again. You can do this by adding a comment to this pull request as follows:
/check-enforcer evaluate
Typically evaulation only takes a few seconds. If you know that your pull request is not covered by a pipeline and this is expected you can override Check Enforcer using the following command:
/check-enforcer override
Note that using the override command triggers alerts so that follow-up investigations can occur (PRs still need to be approved as normal).

What if I am onboarding a new service?

Often, new services do not have validation pipelines associated with them, in order to bootstrap pipelines for a new service, you can issue the following command as a pull request comment:
/azp run prepare-pipelines
This will run a pipeline that analyzes the source tree and creates the pipelines necessary to build and validate your pull request. Once the pipeline has been created you can trigger the pipeline using the following comment:
/azp run js - [service] - ci

deyaaeldeen · 2021-11-10T19:37:05Z

Hey @bzhang0, consider running rush clean to clean any old artifacts first and see if that fixes the issue.

bzhang0 · 2021-11-13T12:14:21Z

At long last, all rush lint errors have been fixed! However, rush rebuild still is not working for me.

==[ BLOCKED: 1 project ]=======================================================

These projects were blocked by dependencies that failed:
  @azure-tests/perf-storage-file-datalake

==[ FAILURE: 3 projects ]======================================================

--[ FAILURE: @azure/storage-file-datalake ]---------[ 6 minutes 4.4 seconds ]--

Invoking: npm run clean && tsc -p . && npm run build:nodebrowser && api-extractor run --local && npm run build:types 


> @azure/[email protected] clean
> rimraf dist dist-* typings temp statistics.html coverage coverage-browser .nyc_output *.tgz *.log test*.xml TEST*.xml


> @azure/[email protected] build:nodebrowser
> rollup -c 2>&1

  ...8 lines omitted...
Error: '@azure/storage-blob' is imported by dist-esm/storage-file-datalake/src/clients.js, but could not be resolved – treating it as an external dependency
    at Object.onwarn (/Users/ben/Documents/repos/azure-sdk-for-js/sdk/storage/storage-file-datalake/rollup.config.js:197:15)
    at Object.onwarn (/Users/ben/Documents/repos/azure-sdk-for-js/common/temp/node_modules/.pnpm/[email protected]/node_modules/rollup/dist/shared/node-entry.js:13599:25)
    at Graph.onwarn (/Users/ben/Documents/repos/azure-sdk-for-js/common/temp/node_modules/.pnpm/[email protected]/node_modules/rollup/dist/shared/node-entry.js:13599:25)
    at Graph.warn (/Users/ben/Documents/repos/azure-sdk-for-js/common/temp/node_modules/.pnpm/[email protected]/node_modules/rollup/dist/shared/node-entry.js:13403:14)
    at ModuleLoader.handleResolveId (/Users/ben/Documents/repos/azure-sdk-for-js/common/temp/node_modules/.pnpm/[email protected]/node_modules/rollup/dist/shared/node-entry.js:12412:24)
    at ModuleLoader.<anonymous> (/Users/ben/Documents/repos/azure-sdk-for-js/common/temp/node_modules/.pnpm/[email protected]/node_modules/rollup/dist/shared/node-entry.js:12298:30)
    at Generator.next (<anonymous>)
    at fulfilled (/Users/ben/Documents/repos/azure-sdk-for-js/common/temp/node_modules/.pnpm/[email protected]/node_modules/rollup/dist/shared/node-entry.js:38:28)


--[ FAILURE: @azure/container-registry ]-----------[ 2 minutes 14.7 seconds ]--

Invoking: npm run clean && tsc -p . && rollup -c 2>&1 && api-extractor run --local 


> @azure/[email protected] clean
> rimraf dist dist-* temp types *.tgz *.log

src/containerRegistryTokenCredential.ts(4,1): error TS6133: 'Mappers' is declared but its value is never read.
src/containerRegistryTokenCredential.ts(5,1): error TS6133: 'Parameters' is declared but its value is never read.
src/containerRegistryTokenCredential.ts(7,10): error TS6133: 'AcrAccessToken' is declared but its value is never read.
src/containerRegistryTokenCredential.ts(7,26): error TS6133: 'AcrRefreshToken' is declared but its value is never read.
src/containerRegistryTokenCredential.ts(8,1): error TS6192: All imports in import declaration are unused.

--[ FAILURE: @azure/cosmos ]-----------------------[ 8 minutes 54.3 seconds ]--


dist-esm/src/index.js → dist/index.js...
[!] Error: Could not resolve entry module (dist-esm/src/index.js).
Error: Could not resolve entry module (dist-esm/src/index.js).
    at error (/Users/ben/Documents/repos/azure-sdk-for-js/common/temp/node_modules/.pnpm/[email protected]/node_modules/rollup/dist/shared/node-entry.js:5400:30)
    at /Users/ben/Documents/repos/azure-sdk-for-js/common/temp/node_modules/.pnpm/[email protected]/node_modules/rollup/dist/shared/node-entry.js:12199:20
    at async Promise.all (index 0)
    at async Promise.all (index 0)
    at async Promise.all (index 0)



Projects failed to build.

rush rebuild (9 minutes 51.3 seconds)

Let me know if I'm missing something!

jeremymeng · 2021-11-16T01:46:00Z

@bzhang0 did you try rush update after merging main?

bzhang0 · 2021-11-16T01:49:35Z

@bzhang0 did you try rush update after merging main?

I will try this!

jeremymeng · 2021-11-17T00:51:35Z

@bzhang0 Since this rule impacts almost every package this PR is becoming too large to manage reviews. We recommend breaking it into smaller ones in phases

First PR to add the rule to the plugin but treat violations as warnings
Then separate PRs for fixing the linter rule in each package.
Once all of them are merged, another PR to change violations from warnings to error

Sorry for the trouble @bzhang0 ! Hopefully you just need to git cherry-pick into new branches then open new PRs.

bzhang0 · 2021-11-17T23:50:19Z

Will do! Just to clarify, you want me to do the following?

make a new branch + pr that adds the rule with warnings
make a new branch + pr for a subdirectory (whose changes can be pulled in from git cherry-pick)
repeat for each subdirectory

additionally, I will merge Azure:main and resolve respective conflicts.

jeremymeng · 2021-12-07T23:51:27Z

Closing this as we will have separate PRs for packages

bzhang0 added 2 commits November 9, 2021 01:39

Add sort-imports rule

a8f1f6d

Updated with respect to sort-imports lint

f79a5ee

bzhang0 requested review from deyaaeldeen, mikeharder, praveenkuttappan and witemple-msft as code owners November 9, 2021 09:45

ghost added eslint plugin customer-reported Issues that are reported by GitHub users external to the Azure organization. labels Nov 9, 2021

Merge branch 'Azure:main' into issue-9252

46e5b0e

updating files

da280d6

bzhang0 requested review from HarshaNalluru, jeremymeng, joheredi, kushagraThapar, maorleger, moderakh, ramya-rao-a, sadasant, simorenoh, simplynaveen20, xinlian12 and xirzec as code owners November 10, 2021 08:52

Merge branch 'main' into issue-9252

1369c2a

bzhang0 marked this pull request as draft November 10, 2021 18:06

bzhang0 requested review from KarishmaGhiya, LarryOsterman, ljian3377, LuChen-Microsoft, MalcolmTyrrell, miguhern, nguerrera, qiaozha, RoyHerrod, sarangan12, schaabs, vicancy, vinjiang, whisper6284, XiaoningLiu, YoDaMa and a team as code owners November 13, 2021 12:00

bzhang0 mentioned this pull request Nov 18, 2021

[Linter] Added sort-imports rule with "warning" #18730

Merged

jeremymeng closed this Dec 7, 2021

bzhang0 mentioned this pull request Dec 8, 2021

[sort-imports] update agrifood-farming-rest with respect to sort-imports rule #19024

Closed

bzhang0 deleted the issue-9252 branch January 16, 2022 19:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Linter] Update files with respect to `sort-imports` rule #18598

[Linter] Update files with respect to `sort-imports` rule #18598

bzhang0 commented Nov 9, 2021 •

edited

Loading

ghost commented Nov 9, 2021

deyaaeldeen commented Nov 9, 2021

bzhang0 commented Nov 9, 2021

bzhang0 commented Nov 9, 2021

check-enforcer bot commented Nov 10, 2021

deyaaeldeen commented Nov 10, 2021

bzhang0 commented Nov 13, 2021

jeremymeng commented Nov 16, 2021

bzhang0 commented Nov 16, 2021

jeremymeng commented Nov 17, 2021

bzhang0 commented Nov 17, 2021 •

edited

Loading

jeremymeng commented Dec 7, 2021

[Linter] Update files with respect to sort-imports rule #18598

[Linter] Update files with respect to sort-imports rule #18598

Conversation

bzhang0 commented Nov 9, 2021 • edited Loading

ghost commented Nov 9, 2021

deyaaeldeen commented Nov 9, 2021

bzhang0 commented Nov 9, 2021

bzhang0 commented Nov 9, 2021

check-enforcer bot commented Nov 10, 2021

What is Check Enforcer?

Why am I getting this message?

What should I do now?

What if I am onboarding a new service?

deyaaeldeen commented Nov 10, 2021

bzhang0 commented Nov 13, 2021

jeremymeng commented Nov 16, 2021

bzhang0 commented Nov 16, 2021

jeremymeng commented Nov 17, 2021

bzhang0 commented Nov 17, 2021 • edited Loading

jeremymeng commented Dec 7, 2021

[Linter] Update files with respect to `sort-imports` rule #18598

[Linter] Update files with respect to `sort-imports` rule #18598

bzhang0 commented Nov 9, 2021 •

edited

Loading

bzhang0 commented Nov 17, 2021 •

edited

Loading