Skip to content

Conversation

@JamesHollyer
Copy link
Contributor

Motivation for features / changes

In #6346 we added a displayName attribute to the ColumnHeader. This PR uses that name to be displayed in the headers.

Technical description of changes

I removed lots of headers from a test since now we do not need to test that each type has the proper text displayed.

@JamesHollyer JamesHollyer requested a review from rileyajones May 3, 2023 16:24
Comment on lines -116 to -127
{
type: ColumnHeaderType.STEP,
name: 'step',
displayName: 'Step',
enabled: true,
},
{
type: ColumnHeaderType.RELATIVE_TIME,
name: 'relativeTime',
displayName: 'Relative',
enabled: true,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason you are removing these from the mocks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No need to test that all of these display the correct text as all it does now is display the displayText. We do not need to test that it displays the displayText that many times.

@JamesHollyer JamesHollyer merged commit 3b51b4f into tensorflow:master May 3, 2023
rileyajones pushed a commit to rileyajones/tensorboard that referenced this pull request May 5, 2023
## Motivation for features / changes
In tensorflow#6346 we added a displayName attribute to the ColumnHeader. This PR
uses that name to be displayed in the headers.

## Technical description of changes
I removed lots of headers from a test since now we do not need to test
that each type has the proper text displayed.
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