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

[deps] Migration to new eslint v9 to flat configuration #1676

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

graduta
Copy link
Member

@graduta graduta commented Jul 24, 2024

I DON'T have JIRA ticket

  • explain what this PR does
  • if it is a new feature, explain how you plan to use it
  • tests are added
  • documentation was updated or added

Notable changes for users:

  • NA

Notable changes for developers:

  • new eslint flat configuration added
  • existing configuration updated to use plugins such as jsdoc/stylistic, etc.

Changes made to the database:

@graduta graduta added the dependencies Pull requests that update a dependency file label Jul 24, 2024
@graduta graduta self-assigned this Jul 24, 2024
Copy link

codecov bot commented Jul 24, 2024

Codecov Report

Attention: Patch coverage is 13.88889% with 31 lines in your changes missing coverage. Please review.

Project coverage is 43.82%. Comparing base (9819518) to head (5923b28).

Files Patch % Lines
lib/public/components/common/markdown/markdown.js 0.00% 4 Missing ⚠️
...ublic/components/Pagination/paginationComponent.js 0.00% 3 Missing ⚠️
...mponents/common/panel/LabelPanelHeaderComponent.js 0.00% 3 Missing ⚠️
...ponents/common/popover/popoverPreConfigurations.js 0.00% 3 Missing ⚠️
...nts/common/selection/dropdown/selectionDropdown.js 0.00% 3 Missing ⚠️
lib/public/utilities/export.js 0.00% 3 Missing ⚠️
...b/public/components/common/panel/PanelComponent.js 0.00% 2 Missing ⚠️
...ublic/components/common/popover/overflowBalloon.js 0.00% 2 Missing ⚠️
...ib/public/views/Logs/Overview/LogsOverviewModel.js 0.00% 2 Missing ⚠️
...ib/public/views/Runs/Overview/RunsOverviewModel.js 0.00% 2 Missing ⚠️
... and 3 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1676      +/-   ##
==========================================
- Coverage   43.90%   43.82%   -0.08%     
==========================================
  Files         872      872              
  Lines       14979    14982       +3     
  Branches     2852     2852              
==========================================
- Hits         6576     6566      -10     
- Misses       8403     8416      +13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment on lines +28 to +29
* @param {Sequalize} sequalize sequalize instance
* @param {Sequalize.Transaction} sequalize.transaction transaction to use
Copy link
Collaborator

Choose a reason for hiding this comment

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

Typo in Sequelize

Comment on lines +38 to +41
/**
* @param {(boolean|{from: boolean, to: boolean})} [configuration.required] defines if the from/to dates are required
* (true means both are required)
* @param {boolean} [configuration.seconds=false] states if the input has granularity up to seconds (if not, granularity is minutes)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure that param make sense out of function signature

@@ -38,7 +38,8 @@ import { StatefulComponent } from '../../StatefulComponent.js';
export class DateTimeInputComponent extends StatefulComponent {
/**
* Constructor
* @property {DateTimeInputComponentVnode} vnode the component's vnode
* @param {DateTimeInputComponentVnode} vnode the component's vnode
* @param {DateTimeInputComponentAttrs} vnode.attrs the component's attributes
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suppose this should not be needed because the type is explicit, not object?

// eslint-disable-next-line require-jsdoc
oncreate: function ({ dom }) {

oncreate: ({ dom }) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you do this, the this in the function will not correspond to the vnode state, which will lead to weird behavior if the function is used in several places at the same time. This applies to all these functions

Comment on lines +179 to +184
/**
* @param {number[]} [options.filter.ids] quality control flag ids to filter with
* @param {boolean} [options.filter.bad] quality control flag boolean to filter bad or not-bad QC flags
* @param {boolean} [options.filter.archived] quality control flag boolean to filter archived or not-archived QC flags
* @param {string[]} [options.filter.names] quality control flag type name to filter with, single token is treated as pattern
* @param {string[]} [options.filter.methods] quality control flag type method to filter with, single token is treated as pattern
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here, not sure the param applies for an object, it might be @type instead

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
Development

Successfully merging this pull request may close these issues.

2 participants