Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RESTWS-676: Improve resource definition properties #405

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gitcliff
Copy link
Contributor

LINK:https://issues.openmrs.org/browse/RESTWS-676
Improve Resource Definition Properties

@@ -98,7 +100,11 @@ public Model getCREATEModel(Representation rep) {
@Override
public Model getUPDATEModel(Representation representation) {
return new ModelImpl()
.property("name", new StringProperty()); //FIXME missing props

Copy link

Choose a reason for hiding this comment

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

I think the extra spaces should be removed on lines 103-104.

import java.util.ArrayList;
import java.util.Collection;
import java.util.List;

Copy link

Choose a reason for hiding this comment

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

I would remove this space

@ghost
Copy link

ghost commented Jan 13, 2020

Also, I was wondering if the rep could be null and how would super.getCREATEmodel(rep) handle it. Overall, I think the code looks clean.

@sherrif10
Copy link
Member

@gitcliff can you improve the spelling of the title please thanks,

@@ -101,7 +103,7 @@ public Model getGETModel(Representation rep) {
.property("name", new StringProperty())
.property("description", new StringProperty())
.property("voided", new StringProperty())
.property("memberIds", new ArrayProperty(new IntegerProperty())); //FIXME
.property("memberIds", new ArrayProperty(new IntegerProperty()));
Copy link
Member

Choose a reason for hiding this comment

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

Here the code seems to look alike

@ibacher ibacher force-pushed the master branch 2 times, most recently from f9c73bb to 2a4407f Compare December 14, 2020 20:10
@corneliouzbett corneliouzbett changed the title RESTWS-676:Ipmrove resource definition properties RESTWS-676: Improve resource definition properties Feb 23, 2022
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.

2 participants