add application properties and a ConfigLoader to load set configurations - #52
Conversation
bamsemats
left a comment
There was a problem hiding this comment.
I think this looks good, nicely done! Might want to implement tests for getPort and getRootDirectory as well, but maybe that's for another PR?
annikaholmqvist94
left a comment
There was a problem hiding this comment.
Nice work! This looks well-structured and adds useful configurability
Clean ConfigLoader implementation with singleton pattern
-Good error handling with meaningful exceptions
Server now accepts configurable port (much more flexible)
-Smart use of getOrDefault for missing config values
As @bamsemats mentioned, tests for getPort() and getRootDirectory(), would be valuable. Could be a follow-up PR.
Overall: Solid implementation that integrates cleanly with existing code
Approved
annikaholmqvist94
left a comment
There was a problem hiding this comment.
Nice work! This looks well-structured and adds useful configurability
Clean ConfigLoader implementation with singleton pattern
-Good error handling with meaningful exceptions
Server now accepts configurable port (much more flexible)
-Smart use of getOrDefault for missing config values
As @bamsemats mentioned, tests for getPort() and getRootDirectory(), would be valuable. Could be a follow-up PR.
Overall: Solid implementation that integrates cleanly with existing code
Approved
29610c8
|
Thanks for the reviews, I figured lets wait with the tests on this class since we will probably update the properties quite a lot going forward, and that means we have to keep updating the tests. |
application-properties.yml: will hold the application configurationsConfigLoader.java: Reads the yaml file and maps the config values to a singleton instance of ConfigLoaderCurrent properties that have been set:
@coderabbitai ignore
closes #50