Skip to content
This repository was archived by the owner on Apr 23, 2021. It is now read-only.

Problem: data property should be a reference#362

Merged
mehdisadeghi merged 8 commits into
masterfrom
revert-data-internalization
Dec 18, 2016
Merged

Problem: data property should be a reference#362
mehdisadeghi merged 8 commits into
masterfrom
revert-data-internalization

Conversation

@mehdisadeghi

Copy link
Copy Markdown
Contributor

Solution: revert corresponding change made in #335

@ahashibon @stefanoborini I reverted a changed which was my bad.

@mehdisadeghi

mehdisadeghi commented Dec 12, 2016

Copy link
Copy Markdown
Contributor Author

@stefanoborini @ahashibon The regenerate brings in also the FACE change (which is removed from cuba.yml)

@ahashibon

Copy link
Copy Markdown
Contributor

Thanks @mehdisadeghi, looks good to me.

@stefanoborini

Copy link
Copy Markdown
Contributor

Ok, now I am confused. Didn't we agree that we had to return a copy?

@stefanoborini

stefanoborini commented Dec 13, 2016

Copy link
Copy Markdown
Contributor

Ok, this PR is doing unrelated things so I will do the following:

  1. update against a new version of simphony-metadata, extracting the relevant bits.
  2. if green, merge it back here.
  3. merge the yapf patch I have already.

On the rest you can discuss it among yourself, and reach a conclusion.

@stefanoborini

Copy link
Copy Markdown
Contributor

I merged master after PR #363 into this one. Now it should just be the "data" change.

@mehdisadeghi

Copy link
Copy Markdown
Contributor Author

@stefanoborini This PR is desired since it provides the expected behavior.

BTW, I realized even simphony v3.0 has been inconsistent regarding data property. Some classes such as Particle treat it as reference and some others like Particles treat it as copy.

@mehdisadeghi mehdisadeghi merged commit c40d04c into master Dec 18, 2016
@stefanoborini stefanoborini deleted the revert-data-internalization branch January 3, 2017 09:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants