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

ci(eslint): ignore private/internal code for jsdoc rules #6416

Merged
merged 9 commits into from
Mar 7, 2023
Merged

ci(eslint): ignore private/internal code for jsdoc rules #6416

merged 9 commits into from
Mar 7, 2023

Conversation

benelan
Copy link
Member

@benelan benelan commented Feb 3, 2023 •
edited by jcfranco
Loading

Related Issue: N/A

Summary

Prevents eslint-jsdoc-plugin from complaining when an interface/function/class/etc is marked as @internal or @private in its jsdoc block. We will need to go through the code and appropriately mark a lot of our utils as @internal.

@benelan benelan requested a review from a team as a code owner February 3, 2023 06:02
@github-actions github-actions bot added the chore Issues with changes that don't modify src or test files. label Feb 3, 2023
@jcfranco
Copy link
Member

jcfranco commented Feb 3, 2023

They would be great if the plugin was smart enough to only warn about public methods, but it's not.

Is it still noisy after setting up the ignorePrivate and ignoreInternal options?

@benelan
Copy link
Member Author

benelan commented Feb 4, 2023 •
edited
Loading

Nice find, not sure how I missed that. Here is a comparison:

  • current number of warnings: 629
  • warnings when using ignorePrivate and ignoreInternal: 589
  • warnings when disabling the three rules: 121

So it's a bit better but still a lot of noise. We could go through and mark all of the util functions as internal I guess

@jcfranco
Copy link
Member

jcfranco commented Feb 7, 2023

We could go through and mark all of the util functions as internal I guess

I think we would want to start doing this eventually. TBH, I'm torn on this one. On the one hand, I'm hesitant to disable valid rules, but on the other, it's hard to spot valid public ones from the rest. Maybe we can coordinate updating all of the internal ones to lessen the noise?

@benelan
ci(eslint): jsdoc rules ignore private/internal blocks
e45d6c3
@benelan
Merge remote-tracking branch 'origin/master' into benelan/eslint-jsdo…
f2d8185 
…c-rulez

* origin/master:
  docs: update component READMEs (#6352)
  ci(next): fix commit message (#6425)
  fix(vite): resolve lazying loading error in dist build (#6436)
  docs(changelog): remove reverted feature (#6435)
  revert: "feat(pagination, split-button, dropdown, date-picker) action-group): add setFocus method (#6405)" (#6426)
  1.1.0-next.2
  docs(segmented-control): update event description for calciteSegmentedControlChange (#6428)
  fix(tree-item): reverses regression to bring back focus when navigating with keyboard (#6424)
  fix(icon): fix icon normalization to handle x-times-named icons (#6422)
@benelan benelan changed the title ci(eslint): disable a few jsdoc rules ci(eslint): ignore private/internal code for jsdoc rules Feb 8, 2023
@benelan
Copy link
Member Author

benelan commented Feb 8, 2023

Maybe we can coordinate updating all of the internal ones to lessen the noise?

Yeah I'm cool with this option. Marking stuff as internal has an effect on the types though, so we will need to be careful. I switched up this PR to ignore private/internal code using the settings you found

@github-actions
Copy link
Contributor

This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions.

@github-actions github-actions bot added the Stale Issues or pull requests that have not had recent activity. label Feb 16, 2023
@benelan benelan merged commit 942cf56 into master Mar 7, 2023
@benelan benelan deleted the benelan/eslint-jsdoc-rulez branch March 7, 2023 20:54
benelan added a commit that referenced this pull request Mar 7, 2023
@benelan
Merge remote-tracking branch 'origin/master' into benelan/delay-tab-s…
c6ada75 
…tory

* origin/master: (57 commits)
  ci(eslint): ignore private/internal code for jsdoc rules (#6416)
  fix(modal): ensure modal transitions are in sync (#6564)
  fix(action): ensure consistent width to accommodate indicator when displaying text (#6562)
  build(deps): Bump focus-trap from 7.2.0 to 7.3.1 (#6540)
  feat(block): add built-in localization (#6503)
  revert(stepper-item): emits calciteStepperItemSelect event when selected (#6560)
  refactor: move ref prop last to ensure ref node is in sync (#6530)
  feat(stepper-item): emits `calciteStepperItemSelect` event when selected. (#6521)
  build(deps): Bump @storybook/addon-a11y from 6.5.15 to 6.5.16 (#6539)
  build(deps): Bump eslint from 8.30.0 to 8.35.0 (#6543)
  chore(block): add t9n message bundles. (#6559)
  build: ensure required files are available for doc preview build (#6557)
  fix(slider): range slider thumb on all touch-enabled devices now follows touch gesture (#6553)
  feat(modal): provides `content-top` and `content-bottom` slots (#6490)
  chore(release): 1.0.8
  chore(release): 1.0.8-next.4
  fix(filter, list): filter properly on initialization (#6551)
  chore(release): 1.0.8-next.3
  fix: apply offsetParent polyfill for Chrome 109+ (#6520)
  fix(tree): restore wrapping in tree-item text content (#6518)
  ...
benelan added a commit that referenced this pull request Mar 8, 2023
@benelan
Merge remote-tracking branch 'origin/master' into benelan/input-incre…
7cddcac 
…ment-bigdecimal

* origin/master:
  test(tabs): delay story due to potential timing chromatic diff (#6437)
  build(deps): Bump chromatic from 6.14.0 to 6.17.1 (#6571)
  build(deps): Bump postcss from 8.4.20 to 8.4.21 (#6570)
  build(deps): Bump eslint-config-prettier from 8.6.0 to 8.7.0 (#6572)
  ci(eslint): ignore private/internal code for jsdoc rules (#6416)
benelan added a commit that referenced this pull request Mar 13, 2023
@benelan
Merge remote-tracking branch 'origin/master' into benelan/support-fir…
cc92f12 
…efox-esr

* origin/master: (62 commits)
  build(deps): Bump rimraf and @types/rimraf (#6541)
  build(deps): Bump quicktype-core from 6.1.0 to 23.0.12 (#6573)
  test(tabs): delay story due to potential timing chromatic diff (#6437)
  build(deps): Bump chromatic from 6.14.0 to 6.17.1 (#6571)
  build(deps): Bump postcss from 8.4.20 to 8.4.21 (#6570)
  build(deps): Bump eslint-config-prettier from 8.6.0 to 8.7.0 (#6572)
  ci(eslint): ignore private/internal code for jsdoc rules (#6416)
  fix(modal): ensure modal transitions are in sync (#6564)
  fix(action): ensure consistent width to accommodate indicator when displaying text (#6562)
  build(deps): Bump focus-trap from 7.2.0 to 7.3.1 (#6540)
  feat(block): add built-in localization (#6503)
  revert(stepper-item): emits calciteStepperItemSelect event when selected (#6560)
  refactor: move ref prop last to ensure ref node is in sync (#6530)
  feat(stepper-item): emits `calciteStepperItemSelect` event when selected. (#6521)
  build(deps): Bump @storybook/addon-a11y from 6.5.15 to 6.5.16 (#6539)
  build(deps): Bump eslint from 8.30.0 to 8.35.0 (#6543)
  chore(block): add t9n message bundles. (#6559)
  build: ensure required files are available for doc preview build (#6557)
  fix(slider): range slider thumb on all touch-enabled devices now follows touch gesture (#6553)
  feat(modal): provides `content-top` and `content-bottom` slots (#6490)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jcfranco jcfranco jcfranco approved these changes

Assignees

@benelan benelan

Labels
chore Issues with changes that don't modify src or test files. Stale Issues or pull requests that have not had recent activity.
Projects
None yet
Milestone
2023 February Priorities - 1.1.0 - No...
Development

Successfully merging this pull request may close these issues.
2 participants
@benelan @jcfranco