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

Feature/149 #23

Closed
wants to merge 2 commits into from
Closed

Feature/149 #23

wants to merge 2 commits into from

Conversation

raamdev
Copy link
Contributor

@raamdev raamdev commented Oct 28, 2015

@raamdev
Copy link
Contributor Author

raamdev commented Oct 28, 2015

@jaswsinc Review please, when you get a moment.

@jaswrks
Copy link

jaswrks commented Oct 28, 2015

Just reviewed those changes. Very nice! Looks great to me.

The only thing I noticed is that there are several whitespace differences (indentation conflicts), but those will get resolved once we address this issue and accomplish PSR 2/4 compat. to match-up with ZenCache and some of our other newer projects.

For now, if you still have PhpStorm and the WebSharks Style Formatter for PhpStorm, you can apply that to the files that you changed. Comment Mail was originally written in PhpStorm with that style. If you don't have that handy, then no worries. We can just merge this and worry about formatting later.

I'll let you do the merging here whenever you feel ready.

@raamdev
Copy link
Contributor Author

raamdev commented Oct 28, 2015

I actually have the WebSharks PSR-1/PSR-2 code style selected in phpStorm. I tried switching back to the old "WebSharks" code style and reformatting the files I changed, but it looks like there are a few formatting things that should not be applied that are getting applied.

I'll just leave things as-is and we can deal with formatting as part of wpsharks/comment-mail#150.

@raamdev raamdev closed this Oct 28, 2015
@raamdev raamdev deleted the feature/149 branch October 28, 2015 03:08
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