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

Issue #326: Fix statically called non-static method. #327

Merged

Conversation

t4k
Copy link
Contributor

@t4k t4k commented Nov 2, 2017

Addresses #326.

Review Procedure

Enable PHP error reporting for E_DEPRECATED and load any CORAL page besides the root index and there will be Deprecated messages. Apply this fix and Deprecated messages for this issue are no longer there.

@remocrevo
Copy link
Contributor

@t4k Watch out for whitespacing (tabs vs. spaces) as seen here:
https://patch-diff.githubusercontent.com/raw/coral-erm/coral/pull/327.diff

@t4k
Copy link
Contributor Author

t4k commented Nov 6, 2017

My interpretation of the discussion at the end of #55 was that we would begin to use the new coding guidelines on all new patches, regardless of the state of the old code, and not update old code just for formatting issues, unless a separate formatting issue is being addressed.

@remocrevo
Copy link
Contributor

I understand the need to not mix coding style changes into an unrelated patch. But in regards to whitespace, as in this case, we are actively changing a single line's indentation style in the middle of a function, which reduces the readability of the code. I don't recall from the discussion, does the new .editorconfig solve this issue?

@t4k
Copy link
Contributor Author

t4k commented Nov 6, 2017

The new .editorconfig does solve the issue locally. It sets tab width to 2, which makes all the indents visually consistent. I’m not sure if we can set GitHub to respect those settings online though.

@veggiematts
Copy link
Contributor

Out of the space-tabs issue, the fix is ok. I would merge this.

@veggiematts
Copy link
Contributor

Moreover, the API is broken without this patch when error_reporting = E_ALL
This is a serious bug, I'm merging this.

@veggiematts veggiematts merged commit aad5c9a into coral-erm:development Jan 29, 2018
@t4k t4k deleted the 326-deprecated-static-method branch January 30, 2018 23:37
@PaulPoulain PaulPoulain added the bug This is a bug (not an enhancement) label Mar 19, 2018
@PaulPoulain PaulPoulain added this to the Version 3.0.0 Beta milestone Mar 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This is a bug (not an enhancement)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants