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

Rename firstThreeColumns #4159

Merged

Conversation

julieg18
Copy link
Contributor

@julieg18 julieg18 commented Jun 21, 2023

2/2 main <= #4155 <= this

  • renames firstThreeColumns (and related variables/keys) to summaryColumns

Followup to #4135 (review)

@@ -282,7 +282,7 @@ describe('ColumnsModel', () => {
)
await model.transformAndSet(outputFixture)

expect(model.getFirstThreeColumnOrder()).toStrictEqual([
expect(model.getSummaryColumnOrder()).toStrictEqual([
Copy link
Contributor Author

@julieg18 julieg18 Jun 21, 2023

Choose a reason for hiding this comment

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

Figured that summaryColumns is simpler than firstThreeMetricAndParamColumns. Plus, if we adjust what these summary columns include we won't need to nename everything again. Though there might be a better term than summary.

@julieg18 julieg18 merged commit dc4c32f into use-metrics-and-params-in-table-tooltips Jun 23, 2023
3 checks passed
@julieg18 julieg18 deleted the rename-first-three-columns branch June 23, 2023 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants