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

Eslint and typescript errors #56

Merged
merged 12 commits into from
Mar 6, 2023
Merged

Eslint and typescript errors #56

merged 12 commits into from
Mar 6, 2023

Conversation

oltionchampari
Copy link
Contributor

@oltionchampari oltionchampari commented Mar 3, 2023 •
edited
Loading

Closes list issues numbers here

Developer Checklist (Definition of Done)

Issue

  • All acceptance criteria from the issue are met
  • Tested in latest Chrome/Firefox

UI/UX/Vis

  • Requires UI/UX/Vis review
    • Reviewer(s) are notified (tag assignees)
    • Review has occurred (link to notes)
    • Feedback is included in this PR
    • Reviewer(s) approve of concept and design

Code

  • Branch is up-to-date with the branch to be merged with, i.e., develop
  • Code is cleaned up and formatted
  • Unit tests are written (frontend/backend if applicable)
  • Integration tests are written (if applicable)

PR

  • Descriptive title for this pull request is provided (will be used for release notes later)
  • Reviewer and assignees are defined
  • Add type label (e.g., bug, feature) to this pull request
  • Add release label (e.g., release: minor) to this PR following semver
  • The PR is connected to the corresponding issue (via Closes #...)
  • Summary of changes is written

Summary of changes

  • Separated Class interfaces from the actual class creation in order to allow import type {IDummy} imports to avoid circular imports.
    In some cases, had to move some util functions that were defined in the wrong places.
  • Fixed all other eslint errors
  • For errors of type Unexpected 'await' inside a loop., i added a eslint-disable flag. Some are intentionally being run in a loop in order for the promises to finish sequentially, in which case the rule should be disabled.
    I added TODOs in any case to evaluate if they need to be refactored, as the amount of effort has to be considered. Refactoring would bring the benefit of the promises running in parallel instead of sequentially, where not done intentially.

Screenshots

Additional notes for the reviewer(s)

Thanks for creating this pull request 🤗

puehringer and others added 10 commits January 12, 2023 22:35
@puehringer
Upgrade python deps (#51)
3e96f3d 
* Upgrade python deps

* Switch back to #develop
@dvvanessastoiber @keckelt
@thinkh @PatrickAdelberger @puehringer
Release 4.1.0 (#54)
6099107 
* prepare next dev version

* Fix colors assignment in plots (#41)

* Prepare github changes

* Remove circleci

* prepare next dev version

* Use `Font Awesome 6 Free` in `font-family` (#39)

* Use `Font Awesome 6 Free` in `font-family`

Requires datavisyn/tdp_core#732

* Update Lineup to 4.6.2

Co-authored-by: Klaus Eckelt <[email protected]>

* Update fontawesome

* Merge visyn_scripts

* prepare next dev version

* Merged `d3_changes` into `develop`  (#45)

* use d3v7 and d3v3 imports
remove depenencies to d3
add dependency to tdp_core

* remove console log

* Dev d3 merge fix (#47)

* move RouterScrollToTop to coral_public

* fix TS errors

* move canvas-confetti to coral_public

* update git dependencies

Co-authored-by: Klaus Eckelt <[email protected]>

* Automatically select root cohort if onboarding was already done (#48)

* Autoselect rootcohort if onboarding was done #579

* format code

* Remove grid lines from visualizations (#49)

remove gridlines #400

* Upgrade python deps (#51)

* Upgrade python deps

* Switch back to #develop

* prepare release 4.1.0

---------

Co-authored-by: Klaus Eckelt <[email protected]>
Co-authored-by: anita-steiner <>
Co-authored-by: Holger Stitz <[email protected]>
Co-authored-by: Patrick <[email protected]>
Co-authored-by: Michael Pühringer <[email protected]>
@dvvanessastoiber
Merge remote-tracking branch 'origin/main' into develop
d5c52f0
@dvvanessastoiber
prepare next dev version
234dd2e
@oltionchampari @puehringer
Migration to visyn_core (#55)
45937d5 
* Migration to visyn_core

* Linting

* Upgrade deps

---------

Co-authored-by: Michael Puehringer <[email protected]>
@oltionchampari
Merge branch 'develop' into och/693-eslint-errors
3d7cf63
@oltionchampari
Fix cohort
7e6931a
@oltionchampari
Fix circular dependencies
d8dbd33
@oltionchampari
Further cleanup
53caad3
@oltionchampari
Fix async in loop
7942a00
@oltionchampari oltionchampari requested a review from thinkh March 3, 2023 10:27
@oltionchampari oltionchampari added type: refactor Refactor the current implementation release: minor PR merge results in a new minor version labels Mar 3, 2023
oltionchampari
oltionchampari commented Mar 3, 2023
View reviewed changes
src/Taskview/tasks/Compare.ts
@@ -220,6 +223,8 @@ export class Compare extends ATask {

for (const [rowIndex, rowCht] of cohorts.entries()) {
// Get the data of 'attr' for the rows inside 'rowCht'
// TODO: fix me
// eslint-disable-next-line no-await-in-loop
const rowData = (await attr.getData(rowCht.dbId)).map((item) => item[attr.dataKey]);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This construct here should maybe be refactored as the linter complains about using await inside a loop which does not look intentional. The code works though.

@puehringer puehringer marked this pull request as ready for review March 6, 2023 11:38
@puehringer puehringer requested a review from keckelt as a code owner March 6, 2023 11:38
@puehringer puehringer merged commit 5222012 into thinkh/693_refactor-and-fix-eslint Mar 6, 2023
@puehringer puehringer deleted the och/693-eslint-errors branch March 6, 2023 11:39
thinkh added a commit that referenced this pull request May 15, 2023
@thinkh @oltionchampari
@puehringer @dvvanessastoiber @keckelt @PatrickAdelberger
refactor: remove dependency cycles to fix eslint errors (#50)
c65c9cb 
* Remove `|| true` to show eslint errors

#693

* Run `lint:fix` to automatically fix formatting

#693

* Add missing component `RouterScrollToTop`

* Refactor src/app.ts into separate files src/app/*

Caleydo/cohort#693

* Rename app/Cohort to app/Coral

* Move colors to config/colors

Caleydo/cohort#693

* Move rest to base/rest

Caleydo/cohort#693

* Move ScrollLinker from util file to utils/ScrollLinker

Caleydo/cohort#693

* Remove unused color schemes

Caleydo/cohort#693

* Extract CohortColorSchema from util to separate file

Caleydo/cohort#693

* Merge CohortColorSchema with config/colors

Caleydo/cohort#693

* Move utilCustomEvents to base/events

Caleydo/cohort#693

* Move utilIdTypes to config/entities

Caleydo/cohort#693

* Move utilLabels to utils/labels

Caleydo/cohort#693

* Merge Tooltip into AColumn and remove export

Caleydo/cohort#693

* Rename event to event2 for nested event

Caleydo/cohort#693

* Merge CohortInterfaces into app/interfaces

Caleydo/cohort#693

* Use `ICohort` instead `Cohort`

Caleydo/cohort#693

- Replace `Cohort` with `ICohort` were possible
- Add interfaces `IInputCohort` and `IOutputCohort`
- Move classes `InputCohort` and `OutputCohort` to Cohort.ts
- Move public functions and properties from `Cohort` to interface `ICohort`
- Move interfaces and enums from `Cohort` to app/interfacoes
- Replace `Array<...>` with `...[]` syntax

* Reorder functions in utlis/labels

Caleydo/cohort#693

* Eslint and typescript errors (#56)

* Upgrade python deps (#51)

* Upgrade python deps

* Switch back to #develop

* Release 4.1.0 (#54)

* prepare next dev version

* Fix colors assignment in plots (#41)

* Prepare github changes

* Remove circleci

* prepare next dev version

* Use `Font Awesome 6 Free` in `font-family` (#39)

* Use `Font Awesome 6 Free` in `font-family`

Requires datavisyn/tdp_core#732

* Update Lineup to 4.6.2

Co-authored-by: Klaus Eckelt <[email protected]>

* Update fontawesome

* Merge visyn_scripts

* prepare next dev version

* Merged `d3_changes` into `develop`  (#45)

* use d3v7 and d3v3 imports
remove depenencies to d3
add dependency to tdp_core

* remove console log

* Dev d3 merge fix (#47)

* move RouterScrollToTop to coral_public

* fix TS errors

* move canvas-confetti to coral_public

* update git dependencies

Co-authored-by: Klaus Eckelt <[email protected]>

* Automatically select root cohort if onboarding was already done (#48)

* Autoselect rootcohort if onboarding was done #579

* format code

* Remove grid lines from visualizations (#49)

remove gridlines #400

* Upgrade python deps (#51)

* Upgrade python deps

* Switch back to #develop

* prepare release 4.1.0

---------

Co-authored-by: Klaus Eckelt <[email protected]>
Co-authored-by: anita-steiner <>
Co-authored-by: Holger Stitz <[email protected]>
Co-authored-by: Patrick <[email protected]>
Co-authored-by: Michael Pühringer <[email protected]>

* prepare next dev version

* Migration to visyn_core (#55)

* Migration to visyn_core

* Linting

* Upgrade deps

---------

Co-authored-by: Michael Puehringer <[email protected]>

* Fix cohort

* Fix circular dependencies

* Further cleanup

* Fix async in loop

* Import

* Add todos for async inside loop errors

---------

Co-authored-by: Michael Pühringer <[email protected]>
Co-authored-by: Vanessa Stoiber <[email protected]>
Co-authored-by: Klaus Eckelt <[email protected]>
Co-authored-by: Holger Stitz <[email protected]>
Co-authored-by: Patrick <[email protected]>
Co-authored-by: dvvanessastoiber <[email protected]>
Co-authored-by: Michael Puehringer <[email protected]>

* bug: merge data subtype object

* Fix client config typings

---------

Co-authored-by: Champari Oltion <[email protected]>
Co-authored-by: Michael Pühringer <[email protected]>
Co-authored-by: Vanessa Stoiber <[email protected]>
Co-authored-by: Klaus Eckelt <[email protected]>
Co-authored-by: Patrick <[email protected]>
Co-authored-by: dvvanessastoiber <[email protected]>
Co-authored-by: Michael Puehringer <[email protected]>
Co-authored-by: oltionchampari <[email protected]>
oltionchampari added a commit that referenced this pull request May 16, 2023
@dvmoritzschoefl @thinkh
@oltionchampari @puehringer @dvvanessastoiber @keckelt @PatrickAdelberger
React 18 Migration (#58)
9e57aec 
* Remove `|| true` to show eslint errors

#693

* Run `lint:fix` to automatically fix formatting

#693

* Add missing component `RouterScrollToTop`

* Refactor src/app.ts into separate files src/app/*

Caleydo/cohort#693

* Rename app/Cohort to app/Coral

* Move colors to config/colors

Caleydo/cohort#693

* Move rest to base/rest

Caleydo/cohort#693

* Move ScrollLinker from util file to utils/ScrollLinker

Caleydo/cohort#693

* Remove unused color schemes

Caleydo/cohort#693

* Extract CohortColorSchema from util to separate file

Caleydo/cohort#693

* Merge CohortColorSchema with config/colors

Caleydo/cohort#693

* Move utilCustomEvents to base/events

Caleydo/cohort#693

* Move utilIdTypes to config/entities

Caleydo/cohort#693

* Move utilLabels to utils/labels

Caleydo/cohort#693

* Merge Tooltip into AColumn and remove export

Caleydo/cohort#693

* Rename event to event2 for nested event

Caleydo/cohort#693

* Merge CohortInterfaces into app/interfaces

Caleydo/cohort#693

* Use `ICohort` instead `Cohort`

Caleydo/cohort#693

- Replace `Cohort` with `ICohort` were possible
- Add interfaces `IInputCohort` and `IOutputCohort`
- Move classes `InputCohort` and `OutputCohort` to Cohort.ts
- Move public functions and properties from `Cohort` to interface `ICohort`
- Move interfaces and enums from `Cohort` to app/interfacoes
- Replace `Array<...>` with `...[]` syntax

* Reorder functions in utlis/labels

Caleydo/cohort#693

* Eslint and typescript errors (#56)

* Upgrade python deps (#51)

* Upgrade python deps

* Switch back to #develop

* Release 4.1.0 (#54)

* prepare next dev version

* Fix colors assignment in plots (#41)

* Prepare github changes

* Remove circleci

* prepare next dev version

* Use `Font Awesome 6 Free` in `font-family` (#39)

* Use `Font Awesome 6 Free` in `font-family`

Requires datavisyn/tdp_core#732

* Update Lineup to 4.6.2

Co-authored-by: Klaus Eckelt <[email protected]>

* Update fontawesome

* Merge visyn_scripts

* prepare next dev version

* Merged `d3_changes` into `develop`  (#45)

* use d3v7 and d3v3 imports
remove depenencies to d3
add dependency to tdp_core

* remove console log

* Dev d3 merge fix (#47)

* move RouterScrollToTop to coral_public

* fix TS errors

* move canvas-confetti to coral_public

* update git dependencies

Co-authored-by: Klaus Eckelt <[email protected]>

* Automatically select root cohort if onboarding was already done (#48)

* Autoselect rootcohort if onboarding was done #579

* format code

* Remove grid lines from visualizations (#49)

remove gridlines #400

* Upgrade python deps (#51)

* Upgrade python deps

* Switch back to #develop

* prepare release 4.1.0

---------

Co-authored-by: Klaus Eckelt <[email protected]>
Co-authored-by: anita-steiner <>
Co-authored-by: Holger Stitz <[email protected]>
Co-authored-by: Patrick <[email protected]>
Co-authored-by: Michael Pühringer <[email protected]>

* prepare next dev version

* Migration to visyn_core (#55)

* Migration to visyn_core

* Linting

* Upgrade deps

---------

Co-authored-by: Michael Puehringer <[email protected]>

* Fix cohort

* Fix circular dependencies

* Further cleanup

* Fix async in loop

* Import

* Add todos for async inside loop errors

---------

Co-authored-by: Michael Pühringer <[email protected]>
Co-authored-by: Vanessa Stoiber <[email protected]>
Co-authored-by: Klaus Eckelt <[email protected]>
Co-authored-by: Holger Stitz <[email protected]>
Co-authored-by: Patrick <[email protected]>
Co-authored-by: dvvanessastoiber <[email protected]>
Co-authored-by: Michael Puehringer <[email protected]>

* bug: merge data subtype object

* React 18 Migration

* Fix typings

* Fix client config typings

* Pin ordino, tourdino

* Add initial value to cohorts array

* Cleanup

* tsconfig

* Update package.json

---------

Co-authored-by: Holger Stitz <[email protected]>
Co-authored-by: Champari Oltion <[email protected]>
Co-authored-by: Michael Pühringer <[email protected]>
Co-authored-by: Vanessa Stoiber <[email protected]>
Co-authored-by: Klaus Eckelt <[email protected]>
Co-authored-by: Patrick <[email protected]>
Co-authored-by: dvvanessastoiber <[email protected]>
Co-authored-by: Michael Puehringer <[email protected]>
Co-authored-by: oltionchampari <[email protected]>
Co-authored-by: Moritz Heckmann <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thinkh thinkh Awaiting requested review from thinkh

@keckelt keckelt Awaiting requested review from keckelt

Assignees
No one assigned
Labels
release: minor PR merge results in a new minor version type: refactor Refactor the current implementation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@oltionchampari @puehringer @dvvanessastoiber