Skip to content

Fix #295#325

Closed
cvgaviao wants to merge 2 commits into
eclipse-vertx:masterfrom
cvgaviao:#295-retry
Closed

Fix #295#325
cvgaviao wants to merge 2 commits into
eclipse-vertx:masterfrom
cvgaviao:#295-retry

Conversation

@cvgaviao

Copy link
Copy Markdown
Contributor

Signed-off-by: Cristiano V. Gavião cvgaviao@gmail.com

Motivation:
Allows the developer to have enum in their data classes that need to convert from and to JSON.
#295

Signed-off-by: Cristiano V. Gavião <cvgaviao@gmail.com>
@vietj vietj added this to the 4.0.4 milestone Mar 17, 2021
@vietj

vietj commented Mar 19, 2021

Copy link
Copy Markdown
Member

can you remove those extra commits for NPE so we can review only the new feature and then we will examine the commits that fix potential NPE

@cvgaviao

Copy link
Copy Markdown
Contributor Author

can you remove those extra commits for NPE so we can review only the new feature and then we will examine the commits that fix potential NPE

done


/**MyEnumWithCustomConstructor doc*/
@VertxGen
public enum MyEnumWithCustomConstructor {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should be renamed with XXXWithCustomFactory because constructor has a very specific meaning

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I did the required changes... :D

But it is really called an enum constructor, not a factory :)
Look here: https://docs.oracle.com/javase/tutorial/java/javaOO/enum.html

import io.vertx.core.json.JsonObject;

@DataObject(generateConverter = true, publicConverter = true)
public class DataObjectWithEnumWithMapper {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DataObjectWithMappedEnum

@vietj

vietj commented Mar 22, 2021

Copy link
Copy Markdown
Member

A couple comments have been made to rename things more consistently.

Signed-off-by: Cristiano V. Gavião <cvgaviao@gmail.com>
@vietj

vietj commented Mar 23, 2021

Copy link
Copy Markdown
Member

I think we need also then to update the index.adoc and the @README` files to somehow say that now enums can be seen as data object when convenient, i.e when the code generator might want to look at this from this perspective.

@vietj

vietj commented Mar 25, 2021

Copy link
Copy Markdown
Member

@cvgaviao can you update the index.adoc and readme files with the changes brought the to model ?

@vietj vietj modified the milestones: 4.0.4, 4.1.0 Mar 30, 2021
@vietj vietj modified the milestones: 4.1.0, 4.1.1 Jun 1, 2021
@vietj vietj modified the milestones: 4.1.1, 4.2.0 Jul 2, 2021
@Fyro-Ing

Copy link
Copy Markdown
Contributor

How can we help to get this feature ?
we could use it with swagger-codegen (after adding a vertx module to generate with @dataobject on project) to have a full OpenAPI contracts compliance & use service proxies or new web api service

@vietj

vietj commented Sep 11, 2021

Copy link
Copy Markdown
Member

this PR is missing update to documentation as said in my last comment, if you can contribute it it is fine

@Fyro-Ing

Copy link
Copy Markdown
Contributor

Update files here Fyro-Ing@1939916

if @cvgaviao can't merge in few days, i'll do a PR directly

Note : Readme updated to conform with json-mappers.properties, not @Mapper

@vietj

vietj commented Sep 13, 2021 via email

Copy link
Copy Markdown
Member

@cvgaviao

Copy link
Copy Markdown
Contributor Author

@Fyro-Ing, you can do that. ;)
I'm not using vert.x in my new job, and the company that I did this PR for also stopped using it.

Update files here Fyro-Ing@1939916

if @cvgaviao can't merge in few days, i'll do a PR directly

Note : Readme updated to conform with json-mappers.properties, not @Mapper

@Fyro-Ing

Copy link
Copy Markdown
Contributor

new PR #341

@vietj vietj closed this Oct 25, 2021
@vietj

vietj commented Oct 25, 2021

Copy link
Copy Markdown
Member

merged with new PR.

@vietj vietj removed this from the 4.2.0 milestone Oct 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants