Skip to content

Conversation

@kirklund
Copy link
Contributor

@kirklund kirklund commented Mar 24, 2021

After Donal removed PowerMock from a test in this package, I made a first pass over cleaning it up further.

  • Extracted some inner-classes to top-level
  • Made some fields final
  • Updated some tests with AssertJ

None of these classes ever go on the wire:

  • CommandResultException -- marked non-serializable field as transient
  • Extracted enum Align from TableBuilder
  • Extracted TooManyColumnsException from TableBuilder and added serialVersionUID

@kirklund kirklund changed the title DRAFT: GEODE-9047: Cleanup package management.internal.cli.result GEODE-9047: Cleanup package management.internal.cli.result Mar 25, 2021
@kirklund kirklund marked this pull request as ready for review March 25, 2021 16:18
@kirklund kirklund requested a review from DonalEvans March 25, 2021 16:29
Copy link
Contributor

@DonalEvans DonalEvans left a comment

Choose a reason for hiding this comment

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

Just a few small suggestions.

@kirklund
Copy link
Contributor Author

@DonalEvans Think of this PR has a first pass cleaning up this package and its tests. I didn't actually write new tests. We can keep cleaning up more in followup PRs.

@kirklund kirklund requested a review from DonalEvans March 25, 2021 19:01
Copy link
Contributor

@mhansonp mhansonp left a comment

Choose a reason for hiding this comment

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

Looks fine.

@kirklund kirklund merged commit b80094e into apache:develop Mar 29, 2021
@kirklund kirklund deleted the GEODE-9047-management-refactor-01 branch March 29, 2021 17:19
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.

5 participants