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

RHOAIENG-14040: Replace 'any' with correct types in typescript code #85

Merged
merged 3 commits into from
Nov 28, 2024

Conversation

caponetto
Copy link

@caponetto caponetto commented Nov 26, 2024

https://issues.redhat.com/browse/RHOAIENG-14040

Although this PR has many changes, this is not a refactoring. It is only meant to replace any with the expected types. It will help to have a safer code for future bug fixes, new features, and refactorings. After this PR, any unexplained attempt to use any will break the build.

A green CI is a good indication that everything is ok, however, a few changes were necessary to be incorporated to work with the types, especially null checks.

Notebook-based images to use for tests (commit 431c88e):

ghcr.io/caponetto/opendatahub-io-elyra/workbench-images:jupyter-trustyai-ubi9-python-3.11-20241126-7650cd8-sha-431c88e9

ghcr.io/caponetto/opendatahub-io-elyra/workbench-images:cuda-jupyter-tensorflow-ubi9-python-3.11-20241126-7650cd8-sha-431c88e9

@jiridanek
Copy link
Member

this is not a refactoring.

Meets Fowler's definition of refactoring, IMO ;P

Refactoring is a controlled technique for improving the design of an existing code base. Its essence is applying a series of small behavior-preserving transformations, each of which “too small to be worth doing”. However the cumulative effect of each of these transformations is quite significant. By doing them in small steps you reduce the risk of introducing errors. You also avoid having the system broken while you are carrying out the restructuring - which allows you to gradually refactor a system over an extended period of time. (https://martinfowler.com/books/refactoring.html)

Adding types in typescript sure is behavior preserving.

@caponetto caponetto marked this pull request as ready for review November 27, 2024 18:21
@caponetto
Copy link
Author

This is ready for review.
As part of my tests, I've found one issue that is reproducing on the latest v4.2.0

@caponetto caponetto merged commit e1a95a6 into opendatahub-io:main Nov 28, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants