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

handle a few 'edge cases' for index lookups #124

Merged
merged 3 commits into from
Nov 23, 2023

Conversation

mpollmeier
Copy link
Contributor

most importantly don't crash and burn if asking for an undefined property,
or for schema-conform property that simply isn't used yet

@mpollmeier mpollmeier requested a review from bbrehm November 22, 2023 15:29
@@ -18,7 +18,7 @@ object DebugDump {
}
}

def debugDump(g: Graph): String = {
def apply(g: Graph): String = {
Copy link
Contributor

Choose a reason for hiding this comment

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

I generally prefer descriptive function names to apply; both for better code reading and for better experience searching for uses. (searching for uses of apply cannot be done via grep, and tends to be slow in intellij).

If you dislike DebugDump.debugDump(g) we could instead import ...DebugDump.debugDump?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, changed to import ...DebugDump.debugDump

this.closed = true

storagePathMaybe.foreach { storagePath =>
println(s"closing graph: writing to storage at `$storagePath`")
Copy link
Contributor

Choose a reason for hiding this comment

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

logger.debug or logger.info? Or //fixme: use proper log message

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i was being lazy, thanks for stopping me. just changed it for slf4j-api

most importantly don't crash and burn if asking for an undefined
property, or for schema-conform property that simply isn't used yet
@mpollmeier mpollmeier force-pushed the michael/index-lookups-handle-edgecases branch from 3340437 to 5f545e4 Compare November 23, 2023 15:41
@mpollmeier mpollmeier merged commit 20537be into master Nov 23, 2023
1 check passed
@delete-merged-branch delete-merged-branch bot deleted the michael/index-lookups-handle-edgecases branch November 23, 2023 15:50
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