Skip to content

Conversation

@bog-walk
Copy link
Member

Description

Summary of the change: Address TODOS involving moving/extracting of internal Exposed objects.

Detailed description:

  • What:
    • All internal SchemaUtilityApi log messages properties moved to companion object
    • Private inner class SchemaUtilityApi.TableDepthGraph moved to separate file as internal class
    • New internal object ExposedMetadataUtils introduced
      • Move internal ExposedDatabaseMetadata methods into that object & adjust usages
    • Move CoreTransactionManager to its own file, along with private class

Type of Change

Please mark the relevant options with an "X":

  • Other - Refactor

- Address multiple todos that mostly don't affect the API too much
@bog-walk bog-walk requested review from e5l and obabichevjb July 18, 2025 04:22
Comment on lines -65 to -66
@Suppress("ForbiddenComment")
// TODO: move to the utility class, rename
Copy link
Member Author

Choose a reason for hiding this comment

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

I believe the intention behind this TODO was that these functions could ultimately be testable in isolation. But I could be recalling wrong.

I'm not sure about the 'renaming' or what the more descriptive name would be? 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hm.. about renaming I'm also not sure what was the particular plan..

Comment on lines +15 to +16
@InternalApi
object ExposedMetadataUtils {
Copy link
Member Author

Choose a reason for hiding this comment

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

As in the previous comment, I left this as public, but marked for internal use, because I think the intention was to eventually be able to test these methods.

If that's not the case, please suggest a different modifier or even a naming change!

Copy link
Member

@e5l e5l left a comment

Choose a reason for hiding this comment

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

lgtm

@bog-walk bog-walk merged commit 1c9f32a into main Jul 25, 2025
5 of 6 checks passed
@bog-walk bog-walk deleted the bog-walk/refactor-files-move-around branch July 25, 2025 02:39
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.

4 participants