-
Notifications
You must be signed in to change notification settings - Fork 0
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
Implementation of Iron library #81
base: main
Are you sure you want to change the base?
Conversation
…set from Earliest to earliest
…ambiguous instances of the same types. Required forcing givens into local scope.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.setAppName(sparkProcessorConfiguration.app.appName.toString) | ||
.setMaster(sparkProcessorConfiguration.app.masterUrl.toString) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
toString
can lead to some issues, as sometimes it is implemented with the hashcode or other stuff. Based on the specification, toString
means a readable object string and shouldn't be treated as a value. Does that type have something like .value
available?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have been iterating on this library at the client and .toString
is also not recommended. There is a better use of the library. I will probably update this PR with the last team agreement. Soon!
This PR seeks to reduce the boilerplate of the app by implementing the
iron
library.The reduction is visible in the total changes:
+168 −769