sdk-java
sdk-java copied to clipboard
serverlessworkflow
Make classes serializable
What this PR does / why we need it: There were some manually created POJOs which did not implement serializable interface. Making those implement Serializable interface here. This will enable Workflow objects transferrable over wire.
@ricardozanini just curious: I saw most of the classes are generated automatically from the JsonSchema but some of them are manually created. These manually created ones did not implement Serializable interface. I wonder why did we create these classes manually?
I have tested this branch (synced with latest main branch) on my local there is no regression and also my usecase is now working after this.
Quick question: Are there any other classes which were manually written apart from the ones present in this package? In that case we will have to make those classes implement serializable too.
NOTE: I am only worried about the classes being used inside the main Workflow class other Util classes are fine.
Quick question: Are there any other classes which were manually written apart from the ones present in this package? In that case we will have to make those classes implement serializable too.
The manually created classes are needed in order to implement a custom behavior that the generated ones cannot.
I'd recommend reviewing the whole project and writing more throughout tests using coverage to make sure everything is set before we merge this one. Can you do it?
Sure. As of now I am using a workaround to solve my usecase so not a blocker for me. It will take some time for me though.
Just a thought: How do you propose testing this in UTs?
Another option is we merge this and I work on a follow up PR which does a comprehensive check as this PR alone seems harmless to me does not cause any regression. Upto you...
I'd rather everything in one PR if you don't mind since I have to cherry pick to another branch to do a fup release.
@visheshruparelia I'm merging this one to help with your use case, sorry for the late merging. We will release a new major version targeting Java 11, so you can go from there.