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

Avoid use of require() in default-config.tsx #1225

Merged
merged 2 commits into from
Mar 1, 2023

Conversation

mszabo-wikia
Copy link
Contributor

Which problem is this PR solving?

Short description of the changes

This needs to be an import for an eventual build system switch, since Vite uses ES modules. It also helps make things more consistent.

The UI package version in the About Jaeger dropdown still shows up.

yurishkuro
yurishkuro previously approved these changes Mar 1, 2023
@yurishkuro
Copy link
Member

[tsc-lint     ] error TS6307: File '/home/runner/work/jaeger-ui/jaeger-ui/packages/jaeger-ui/package.json' is not in project file list. Projects must list all files or use an 'include' pattern.

This needs to be an import for an eventual build system switch,
since Vite uses ES modules. It also helps make things more consistent.

The UI package version in the About Jaeger dropdown still shows up.

Signed-off-by: Máté Szabó <mszabo@fandom.com>
@mszabo-wikia
Copy link
Contributor Author

Oops, fixed.

@yurishkuro
Copy link
Member

lots of other lint errors

@mszabo-wikia
Copy link
Contributor Author

That's odd... the test run failed but there's no test marked as explicitly failed, perhaps the job itself got killed off.

The noise in tests is from the Enzyme adapter, since that one is originally for React 17 and with Enzyme being effectively dead, there's no React 18 adapter as far as I'm aware, hence the warnings logged about it using a deprecated API. It's something to look into as a next step.

The tests do pass for me locally. I'll push an empty commit to re-trigger them and see if that helps.

Signed-off-by: Máté Szabó <mszabo@fandom.com>
@mszabo-wikia
Copy link
Contributor Author

Amusingly, it seems Codecov crashes on empty commits: https://app.codecov.io/github/jaegertracing/jaeger-ui/commit/984f8a24b2e11d3d214a876d50f5a179650218d5

However, the retry seems to have helped.

@yurishkuro yurishkuro merged commit f5d276d into jaegertracing:main Mar 1, 2023
@yurishkuro
Copy link
Member

🎉

Binrix pushed a commit to Binrix/jaeger-ui that referenced this pull request Apr 18, 2023
## Which problem is this PR solving?
- Split from jaegertracing#1212

## Short description of the changes
This needs to be an import for an eventual build system switch, since
Vite uses ES modules. It also helps make things more consistent.

The UI package version in the About Jaeger dropdown still shows up.

---------

Signed-off-by: Máté Szabó <mszabo@fandom.com>
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.

2 participants