Skip to content

Conversation

@Duologic
Copy link
Contributor

@Duologic Duologic commented Jan 5, 2024

This PR rewrites util.grid.makeGrid() to use the helper functions from util.panel introduced in #155, this could serve as an example on how to use these functions to create other grid builders.

This PR can be reviewed commit-by-commit, following a proper TDD approach:

  1. introduce unit tests, this surfaces a few minor bugs in the current makeGrid() (see failing tests)
  2. fixes the current makeGrid() to pass the unit tests
  3. fixes runtimeDashboard smoke tests to match this change in behavior
  4. the actual refactor, this time only the smoke tests fail
  5. fixes redMethod smoke test, this test surfaced a bug in the refactor
  6. fixes runtimeDashboard smoke test, this test surfaced a bug in the current makeGrid(), incorrectly calculating the Y for collapsed row panels

@Duologic Duologic force-pushed the duologic/refactor_makegrid_1 branch from b1835ab to 57a407e Compare January 5, 2024 09:15
@Duologic Duologic force-pushed the duologic/refactor_makegrid_1 branch from 57a407e to 79fe602 Compare January 5, 2024 09:17
@Duologic Duologic marked this pull request as ready for review January 5, 2024 10:21
@Duologic Duologic requested a review from a team January 5, 2024 10:21
"w": 8,
"x": 0,
"y": 10
"y": 9
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is a refactor, how come your output has changed?

Copy link
Contributor Author

@Duologic Duologic Jan 5, 2024

Choose a reason for hiding this comment

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

This happens in commit 3, before the refactoring. Commit 2 fixes a few bugs in makeGrid, these fixes change the output. The refactor happens in commit 4.

Only commit 6 is a change of the output that I didn't fix in commit 2, I'm being pragmatic here.

Also see description of the PR.

@Duologic Duologic merged commit c13ba17 into main Jan 5, 2024
@Duologic Duologic deleted the duologic/refactor_makegrid_1 branch January 5, 2024 12:42
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