Skip to content

Core pr 3 (merge with Artesis)#2

Merged
kasperg merged 32 commits intomasterfrom
unknown repository
Jul 3, 2013
Merged

Core pr 3 (merge with Artesis)#2
kasperg merged 32 commits intomasterfrom
unknown repository

Conversation

@cableman
Copy link
Copy Markdown
Member

@cableman cableman commented May 7, 2013

This repository have been merged with the Artesis libraries.

  • Updated to use virtual fields from black hole.
  • Updated coding style.

pjohans and others added 30 commits June 8, 2012 11:41
We want this to be true:
$first = empty($entity->var);
$entity-var;
$first === empty($entity->var);

Which it wouldn't be before this change, if there was a getVar() method.
…d _field_invoke_multiple in field_attach_prepare_view will go through all field and not only the ones specified in the view mode.
Comment thread ding_entity.module
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.

Why have such debugging information been removed?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'd rather ask, why has it ever been committed? I don't think we want Drupal throwing backtraces in production.

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.

This only occurs if code hits an undefined property of a DingEntityBase object. Since it implements magic gettings we do not get warnings for undefined properties as would normally be the case for standard PHP objects.

It is primarily meant as a tool for developers. We should not reach this in production.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't known why this have been removed, but I think that we should never have backtrace and trigger error in production code.

On another node I'm personally not to happy with the DingEntityBase object, but that might be because I don't know the reason behind it and the way it have been created. It just seams that all "hard" problems/errors always ends up in DingEntityBase.

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.

Of course we should not trigger ellers or have backtraces in production code but PHP will trigger errors if code tries to access an undefined property on an object no matter where it is run. It is the developers' responsibility to ensure that it does not happen.

The code above does that change that.

@mikl
Copy link
Copy Markdown

mikl commented Jun 10, 2013

Looks good to me 👍

@kasperg
Copy link
Copy Markdown
Member

kasperg commented Jun 19, 2013

Ready for merge.

The comments regarding debugging code is interesting but should not block progress.

@kasperg
Copy link
Copy Markdown
Member

kasperg commented Jul 3, 2013

Merged.

kasperg added a commit that referenced this pull request Jul 3, 2013
Core pr 3 (merge with Artesis)
@kasperg kasperg merged commit 0d1f235 into ding2:master Jul 3, 2013
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.

7 participants