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

feat: upgrade to Patternfly v5 #953

Merged
merged 6 commits into from
May 30, 2024
Merged

Conversation

mmelko
Copy link
Collaborator

@mmelko mmelko commented May 21, 2024

I used @patternfly/pf-codemods to upgrade most of the things, then fixed errors and some warnings. And upgraded some tables. In the next iteration I will ged rid of the rest deprecated components, mainly Selects and Dropdowns.
related to #477
branch: jsolovjo:e2e-fix-patternfly5-tests

@mmelko mmelko changed the title Patternfly 5 upgrade feat: upgrade to Patternfly v5 May 21, 2024
Copy link

github-actions bot commented May 21, 2024

Test Results

  8 files  ±0    8 suites  ±0   41m 55s ⏱️ - 9m 25s
 65 tests ±0   63 ✅ ±0   2 💤 ±0  0 ❌ ±0 
528 runs  ±0  500 ✅ ±0  28 💤 ±0  0 ❌ ±0 

Results for commit fcfd77b. ± Comparison against base commit 7e100a1.

♻️ This comment has been updated with latest results.

Copy link
Member

@tadayosi tadayosi left a comment

Choose a reason for hiding this comment

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

the changes too big. first review batch.

app/src/examples/example3/Example3HeaderItems.tsx Outdated Show resolved Hide resolved
packages/hawtio/package.json Outdated Show resolved Hide resolved
packages/hawtio/src/core/event-service.ts Show resolved Hide resolved
packages/hawtio/src/plugins/camel/CamelPreferences.tsx Outdated Show resolved Hide resolved
packages/hawtio/src/plugins/camel/CamelPreferences.tsx Outdated Show resolved Hide resolved
packages/hawtio/src/plugins/shared/chart/Chart.tsx Outdated Show resolved Hide resolved
@grgrzybek
Copy link
Contributor

Thanks @tadayosi for careful review - I admit, I simply assumed Matej did the best.

Copy link
Member

@tadayosi tadayosi left a comment

Choose a reason for hiding this comment

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

finished review. at this point I'd like to request changes, as the way you changed around the curried callback functions is alarming and suspected that it breaks the code. please check them again.

packages/hawtio/src/plugins/connect/ConnectionStatus.tsx Outdated Show resolved Hide resolved
packages/hawtio/src/plugins/connect/ConnectionStatus.tsx Outdated Show resolved Hide resolved
packages/hawtio/src/plugins/connect/ConnectionStatus.tsx Outdated Show resolved Hide resolved
packages/hawtio/src/ui/login/HawtioLogin.tsx Outdated Show resolved Hide resolved
packages/hawtio/src/ui/page/HawtioPage.tsx Outdated Show resolved Hide resolved
@hawtio-ci
Copy link

hawtio-ci bot commented May 21, 2024

Test results

Run attempt: 1797
Detailed summary

NAME TESTS PASSED ✅ SKIPPED 💤 FAILED ❌ ERRORS 🚫 TIME 🕖
results-quarkus-node(18)-java(17)-firefox 66 63 3 0 0 319.333
results-quarkus-node(18)-java(21)-firefox 66 63 3 0 0 310.051
results-quarkus-node(20)-java(17)-firefox 66 63 3 0 0 311.642
results-quarkus-node(20)-java(21)-firefox 66 63 3 0 0 310.963
results-springboot-node(18)-java(17)-firefox 66 62 4 0 0 311.598
results-springboot-node(18)-java(21)-firefox 66 62 4 0 0 318.419
results-springboot-node(20)-java(17)-firefox 66 62 4 0 0 319.127
results-springboot-node(20)-java(21)-firefox 66 62 4 0 0 314.764

@mmelko
Copy link
Collaborator Author

mmelko commented May 23, 2024

Thanks @tadayosi for careful review - I admit, I simply assumed Matej did the best.

Well I did:), or at least I tried diven that time and space ;)

@mmelko
Copy link
Collaborator Author

mmelko commented May 23, 2024

finished review. at this point I'd like to request changes, as the way you changed around the curried callback functions is alarming and suspected that it breaks the code. please check them again.

Thanks for the review, as I mentioned you're right, I missed curried signatures of the functions. It also discovered other thing, and that is that are unit test suite could be better too.

@mmelko mmelko force-pushed the patternfly-5-upgrade branch from b177359 to dcf0570 Compare May 28, 2024 13:22
@mmelko mmelko force-pushed the patternfly-5-upgrade branch from dcf0570 to bdc8091 Compare May 28, 2024 13:27
Copy link
Member

@tadayosi tadayosi left a comment

Choose a reason for hiding this comment

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

Resolved some and added/updated some more comments

app/src/examples/example3/Example3HeaderItems.tsx Outdated Show resolved Hide resolved
packages/hawtio/src/plugins/shared/chart/Chart.tsx Outdated Show resolved Hide resolved
packages/hawtio/src/plugins/shared/chart/Chart.tsx Outdated Show resolved Hide resolved
packages/hawtio/src/plugins/springboot/Health.tsx Outdated Show resolved Hide resolved
packages/hawtio/src/plugins/springboot/Health.tsx Outdated Show resolved Hide resolved
packages/hawtio/src/plugins/springboot/Health.tsx Outdated Show resolved Hide resolved
packages/hawtio/src/plugins/springboot/TraceView.tsx Outdated Show resolved Hide resolved
packages/hawtio/src/plugins/springboot/TraceView.tsx Outdated Show resolved Hide resolved
packages/hawtio/src/ui/login/HawtioLogin.tsx Outdated Show resolved Hide resolved
packages/hawtio/src/ui/page/HawtioPage.tsx Outdated Show resolved Hide resolved
…ction signatures in the OperationForm and CamelPreferences

testing-library version change,
removal of unnecessary comments,
change icons color to status definitions
delete jpeg barckgrounds and use svg instead, etc
@mmelko mmelko force-pushed the patternfly-5-upgrade branch from df1ecb1 to fcfd77b Compare May 30, 2024 09:27
@tadayosi tadayosi merged commit f7f3f8b into hawtio:main May 30, 2024
13 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