Skip to content

Conversation

@wendigo
Copy link
Contributor

@wendigo wendigo commented Jun 23, 2025

Fixes #26031

Description

Additional context and related issues

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

## UI
* Improve UI rendering when Trino cluster is air-gapped ({issue}`26031`)

@nadavtzaysler
Copy link
Contributor

Why was the pr before this supersede?

@wendigo
Copy link
Contributor Author

wendigo commented Jun 23, 2025

@nadavtzaysler it's a simpler implementation

@nadavtzaysler
Copy link
Contributor

nadavtzaysler commented Jun 23, 2025

@wendigo It's a simple change in the css file, could've been made in the same pr

Also woof2 files are considered better to use in web apps so i believe the implementation before is more ideal

@wendigo wendigo force-pushed the serafin/local-fonts branch 3 times, most recently from d3867fb to 69dd2c5 Compare June 25, 2025 12:57
@wendigo wendigo force-pushed the serafin/local-fonts branch from 69dd2c5 to 264c850 Compare June 25, 2025 13:05
@nadavtzaysler
Copy link
Contributor

@wendigo @mosabua I still dont understand why is this simple remove of a few lines in the fonts.css file is requiring a new pr, basically until 5 minutes ago it was just a worse copy of the original pr that will take about 3 to 4 more days to merge while the original was ready

@wendigo
Copy link
Contributor Author

wendigo commented Jun 25, 2025

@nadavtzaysler this PR isn't a copy, I've made it from scratch addressing the linked issue

@wendigo wendigo force-pushed the serafin/local-fonts branch from 264c850 to 2012a9f Compare June 25, 2025 13:23
@mosabua
Copy link
Member

mosabua commented Jun 25, 2025

I dont mind which PR is merged - whichever is better. I think we should just add a co authored by line in the commit message so the changes are attributed to both @nadavtzaysler and @wendigo

@wendigo
Copy link
Contributor Author

wendigo commented Jun 25, 2025

Since CLA is not signed I can't add authorship and as said these are two separate PRs addressing the same issue with two different and distinct implementations @mosabua

@wendigo wendigo merged commit 0419b43 into master Jun 25, 2025
99 of 103 checks passed
@wendigo wendigo deleted the serafin/local-fonts branch June 25, 2025 15:32
@github-actions github-actions bot added this to the 477 milestone Jun 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

Using Trino without internet connection will cause slowness in the UI

5 participants