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

add Instant default for config #485

Merged
merged 1 commit into from
Nov 19, 2024
Merged

Conversation

webecke
Copy link
Contributor

@webecke webecke commented Nov 19, 2024

Some silly goose me forgot to set up the default value of an Instant in the Configuration SQL dao. So that meant it would crash when starting up. This fixes that bug currently on the main branch that came from #466

Guess I need to do more thorough testing

@webecke webecke requested a review from ThanGerlek November 19, 2024 21:15
@webecke webecke self-assigned this Nov 19, 2024
@webecke webecke added bug Something isn't working important This has been deemed important by the professors for immediate attention labels Nov 19, 2024
Copy link
Contributor

@frozenfrank frozenfrank left a comment

Choose a reason for hiding this comment

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

Looks good. Typescript has ways to protect against this kind of error, but is there a way to identify it at compile time with Java?

@webecke webecke merged commit 206bb1f into main Nov 19, 2024
2 checks passed
@webecke webecke deleted the dallin-goofed-config-defaults branch November 19, 2024 21:47
@ThanGerlek
Copy link
Contributor

ThanGerlek commented Nov 19, 2024

Not easily, it seems. The simplest way seems to be to simply overload all the relevant functions, which would let us get rid of that generic entirely, but that would mean modifying the ConfigurationDao interface and every relevant implementation whenever we add a new type. Or create wrapper classes for every type which also seems like a bit much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working important This has been deemed important by the professors for immediate attention
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants