Skip to content

Hide hardware information in build log viewer drawer #1284

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

Closed
wants to merge 2 commits into from

Conversation

Keerthi421
Copy link

What does this PR do?

->Hides hardware information in the log viewer for build logs.
->Keeps hardware information visible for test logs.
->Cleans up the UI by removing unnecessary details for builds.

How was this done?

->Updated LogViewerCard to only show hardware info when the log type is not 'build'.

Closes #1283

Copy link
Contributor

@padovan padovan left a comment

Choose a reason for hiding this comment

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

Hello! Thanks for your contribution.

Your change to remove the info from the log viewer seems correct to me (@MarceloRobert will check it in detail).

However we have to clean up this PR a bit. You add the package-lock.json and routetree.gen.ts files in, but they seem unrelated to the change we need to fix the issue.

Thanks again for your patch!!

Copy link
Collaborator

Choose a reason for hiding this comment

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

No need to change routeTree gen. Make sure you have the updated libraries by running pnpm install in the frontend directory since this is an automatically generated file.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The change seems to be ok, but when there is no log to be shown the "No logs available" is getting right after the commit hash. You can change the span component above to a div and they will be in the correct position, try to keep the bottom margin as well, it's helpful to separate the information.

image

@MarceloRobert
Copy link
Collaborator

I believe that the CI will fail but don't worry about it, we are still going to improve the way that GitHub handles forks (issue #1181)

Copy link
Collaborator

@MarceloRobert MarceloRobert left a comment

Choose a reason for hiding this comment

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

Some changes, and you still don't need to change package-lock and routeTree.gen

@@ -95,21 +95,23 @@ export const LogViewerCard = ({
) : (
<>
<div>
<span className="font-medium">
<div className="font-medium mb-2">
Copy link
Collaborator

Choose a reason for hiding this comment

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

In the way that you made it now, there is a double margin on log viewers with the hardware text. Check the placement of the components, try to keep the styling similar with or without the hardware text.

image

Copy link
Author

Choose a reason for hiding this comment

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

@MarceloRobert , I will make required changes

@MarceloRobert
Copy link
Collaborator

Also, sorry if my last comment was confusing but please do remove the poetry-lock and routeTree changes

@MarceloRobert
Copy link
Collaborator

This PR will be closed for being stale. The issue will be continued on PR #1313

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.

Remove hardware information on build drawer
3 participants