Remove usage Format Json from BaseFaker - #663
Conversation
|
| JsonTransformer.JsonTransformerBuilder<Object> jsonTransformerBuilder = new JsonTransformer.JsonTransformerBuilder<>(); | ||
| JsonTransformer<Object> transformer = jsonTransformerBuilder.build(); |
There was a problem hiding this comment.
I guess it could be moved to static final
| JsonTransformer.JsonTransformerBuilder<Object> jsonTransformerBuilder = new JsonTransformer.JsonTransformerBuilder<>(); | ||
| JsonTransformer<Object> transformer = jsonTransformerBuilder.build(); |
There was a problem hiding this comment.
I guess it could be moved to static final
|
Hi @RVRhub |
| data.add(apply(in, schema)); | ||
| Iterator<IN> iterator = input.iterator(); | ||
| while (iterator.hasNext()){ | ||
| data.add(apply(iterator.next(), schema) + (commaBetweenObjects & iterator.hasNext() ? "," : "")); |
| field("unit", Data::unit) | ||
| )); | ||
|
|
||
| System.out.println(json); |
There was a problem hiding this comment.
I think we could remove this output
| } | ||
|
|
||
| public Json jsona(String... fieldExpressions) { | ||
| public String jsona(String... fieldExpressions) { |
There was a problem hiding this comment.
Are these considered breaking changes? Since they’re public methods shouldn’t they be deprecated and alternatives added/encouraged?
There was a problem hiding this comment.
I guess this question applies for FakerValueService too
There was a problem hiding this comment.
Normally I'd say yes, but I think the usage is so limited that the impact would be minimal. I would prefer depreciating a method, but it might be tricky in this case since only the return type changed. But I thought Json was already deprecated?
There was a problem hiding this comment.
I didn’t check in an IDE. This is just me reading things and trying to get used to how the project does things 😉
There was a problem hiding this comment.
yes, Json and all other classes from formats package were deprecated with corresponding comments.
However it is a fair point that probably in future we should deprecate methods and probably some other usages as well ....
There was a problem hiding this comment.
The idea was to make it failing for deprecated code which was deprecated in previous releases. However it should not fail for the code which was deprecated only in current development cycle.
Only pick each warning/failure and compare against git logs manually
There was a problem hiding this comment.
Okay if that's what you're after then yes it isn't something we can do.
We can limit to deprecation concerns -Xlint:deprecation, but the catch is that whenever we deprecate something we'd also have to stop using it internally. Which really should be true anyway IMHO but isn't necessarily quick or straight forward.
On that limited front here's what we'd have to address:
[WARNING] /C:/datafaker/src/main/java/net/datafaker/service/FakeValuesService.java:[447,20] net.datafaker.formats.Csv in net.datafaker.formats has been deprecated
[WARNING] /C:/datafaker/src/main/java/net/datafaker/service/FakeValuesService.java:[447,43] net.datafaker.formats.Csv in net.datafaker.formats has been deprecated
[WARNING] /C:/datafaker/src/main/java/net/datafaker/service/FakeValuesService.java:[457,9] net.datafaker.formats.Csv in net.datafaker.formats has been deprecated
[WARNING] /C:/datafaker/src/main/java/net/datafaker/service/FakeValuesService.java:[457,36] net.datafaker.formats.Csv in net.datafaker.formats has been deprecated
[WARNING] /C:/datafaker/src/main/java/net/datafaker/service/FakeValuesService.java:[460,30] net.datafaker.formats.Csv in net.datafaker.formats has been deprecated
[WARNING] /C:/datafaker/src/main/java/net/datafaker/service/FakeValuesService.java:[462,16] net.datafaker.formats.Format in net.datafaker.formats has been deprecated
[WARNING] /C:/datafaker/src/main/java/net/datafaker/transformations/JavaObjectTransformer.java:[20,31] newInstance() in java.lang.Class has been deprecated
There was a problem hiding this comment.
yes, probably we can go that way, agree.
There was a problem hiding this comment.
Okay. I’ll open an issue. I started to look at some of these. But my lack of familiarity with the code base quickly became a challenge 😉
There was a problem hiding this comment.
From one side this would be great.
From another side just keep in mind that @RVRhub is working on removal of Format.Json
kingthorin
left a comment
kingthorin
left a comment
There was a problem hiding this comment.
Seems reasonable to me
|
@RVRhub thanks for addressing comments |
4 participants
No description provided.