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

Default values prepopulation. For issue #4535 #4552

Merged

Conversation

PhantomYdn
Copy link
Contributor

No description provided.

@lvca
Copy link
Member

lvca commented Jul 27, 2015

WDYT about pre-populate them on setClass() and setClassname() ?

@lvca lvca self-assigned this Jul 27, 2015
@PhantomYdn
Copy link
Contributor Author

In this case I personally don't like a side effect: if class was set twice - class will have default values also from first class populated...

@lvca
Copy link
Member

lvca commented Aug 6, 2015

You're right, but semantically these 2 calls should produce the same effect:

new ODocument("Person");
new ODocument().setClassname("Person");

…default-values-prepopulation

Conflicts:
	core/src/main/java/com/orientechnologies/orient/core/record/impl/ODocument.java
@PhantomYdn
Copy link
Contributor Author

@lvca agreed! PR has been updated with recent code. Please take a look into testcase for all supported cases.

@PhantomYdn
Copy link
Contributor Author

Guys? Build is failing due to timeout. Local test is fine.

@lvca
Copy link
Member

lvca commented Aug 24, 2015

Perfect, merging it now. Thanks.

@lvca lvca closed this Aug 24, 2015
@lvca lvca reopened this Aug 24, 2015
lvca added a commit that referenced this pull request Aug 24, 2015
@lvca lvca merged commit 7139b3a into orientechnologies:develop Aug 24, 2015
@lvca lvca added this to the 2.2 milestone Aug 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants