Issue30 builderclasses

[muralibasani]

About this change - What it does

Introduces enums, valid requests between apis

Resolves: Removes clumsy code
Why this way : Easy maintenance, robust code

[mdedetrich]

Looks good to me, albeit one comment regarding the response in case of exception/internal server error.

[jlprat]

I have some change requests.
And a general comment. We should avoid having a 140 file PR with over 9000 lines added/modified. I see there are at least 3 or 4 set of changes in this PR. Next time let's try to separate changes in smaller PRs

[mdedetrich]

@jlprat Regarding your comments about formatting, does it make sense to incorporate a java formatter into this project to automatically handle these things? Something like https://github.com/diffplug/spotless should do the trick, and its better to do these things sooner rather than later.

[mdedetrich]

So my change was done so approval from my end. I do agree with Josep thought that splitting up the PR's into smaller chunks if possible is much more desirable unless the changes are entirely mechanical (i.e. refactoring).

[jlprat]

@jlprat Regarding your comments about formatting, does it make sense to incorporate a java formatter into this project to automatically handle these things? Something like https://github.com/diffplug/spotless should do the trick, and its better to do these things sooner rather than later

Yes, I think we'd need to create an issue for this. I don't know if Spotless or another one more Java-native.

[mdedetrich]

So spotless itself doesn't do formatting, it just an abstraction around existing formatters (with Java you have a choice of like 4 including ones such as https://github.com/google/google-java-format) that also adds other options like ratcheting.

In any case we should discuss this on an separate issue

[muralibasani]

@jlprat Regarding your comments about formatting, does it make sense to incorporate a java formatter into this project to automatically handle these things? Something like https://github.com/diffplug/spotless should do the trick, and its better to do these things sooner rather than later.

@jlprat @mdedetrich We have spotless already in there.

[muralibasani]

So my change was done so approval from my end. I do agree with Josep thought that splitting up the PR's into smaller chunks if possible is much more desirable unless the changes are entirely mechanical (i.e. refactoring).

@mdedetrich totally agree, there were several inter related changes. And even tried to change few unrelated html content.

[mdedetrich]

@jlprat @mdedetrich We have spotless already in there.

In that case we should pick a suitable style of formatting and just accept it, the major point of having automated formatters is to prevent bikeshedding arguments over style as well as to provide a consistent look for the project.

Something to consider for later is to add a github workflow action to check that the codebase is formatted properly.

[muralibasani]

So spotless itself doesn't do formatting, it just an abstraction around existing formatters (with Java you have a choice of like 4 including ones such as https://github.com/google/google-java-format) that also adds other options like ratcheting.

In any case we should discuss this on an separate issue

@mdedetrich indeed, we have this configured



${google-java-format.version}
<style>GOOGLE</style>

[jlprat]

LGTM