Fixes #28052: Add CSV common datastructures in rudder as utils #6808
Conversation
clarktsiory
force-pushed
the
ust_28052/add_csv_common_datastructures_in_rudder_as_utils
branch
from
8a9496e to
aa40860
Compare
|
Commit modified |
1 similar comment
|
Commit modified |
clarktsiory
force-pushed
the
ust_28052/add_csv_common_datastructures_in_rudder_as_utils
branch
from
aa40860 to
b54c827
Compare
clarktsiory
force-pushed
the
ust_28052/add_csv_common_datastructures_in_rudder_as_utils
branch
from
b54c827 to
3a3f358
Compare
|
Commit modified |
|
You are missing unit tests :) Especially on a feature that wills to become generic up to standard HTTP response |
|
@fanf I will add examples of tests for the whole Also, for the REST util methods, they will need to be based on sample datastructures too, so I'll be it adding at the same time too |
fanf
left a comment
fanf
left a comment
There was a problem hiding this comment.
At minima, you need to remove the now useless dependency in rudder/rudder-templates/pom.xml
| </dependency> | ||
| <dependency> | ||
| <groupId>org.apache.commons</groupId> | ||
| <artifactId>commons-lang3</artifactId> |
There was a problem hiding this comment.
is there a lot of work to remove commans-lang if we have commons-lang3 ?
commons-langis inpom.xml- we aleady have
commons-lang3inrudder/rudder-templates/pom.xml(so we should already have done that work likely)
2 participants
https://issues.rudder.io/issues/28052
Moving the type-classes for CSV from a private plugins to
utils, since we would need it other places than pluginsThe changes are about :
DateTime,ZonedDateTime)ProductDerivationin magnolia, since we don't know how to derive "sum" types (CSV model maps to case class which can justderivean instance, and sealed trait instances will need to be defined manually)