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

Modified logic to avoid returning boundary in HTMLBody #2

Merged
merged 3 commits into from
Mar 9, 2016

Conversation

ibrahimlawal
Copy link

  • Modified logic to make sure that boundary does not get added to returned HTMLBody.
  • Added .gitignore
  • Ran did PSR-2 check on files

@djmattyg007
Copy link
Owner

Thanks for re-submitting the pull request. I had a couple more notes that I missed the first time around:

  • The latest version of the code sniffer library is 2.5.1, so it doesn't make a lot of sense to start off by allowing an out of date version (~2.3)
  • The rest of the library uses camelCase, not snake_case, so please rename the boundaries_raw variable
  • No tests have been added or updated as part of this change. I'd strongly suggest adding at least one test so I know what sort of situation this is catering for.

FInally, please don't use markdown in your commit messages. It looks ugly on both github and (more importantly) on the command line.

@ibrahimlawal
Copy link
Author

Requested commits added

@ibrahimlawal
Copy link
Author

Hi Matthew,

I haven't heard from you since my last commits. Hope you're good.

Thanks!

@djmattyg007
Copy link
Owner

Sorry, I've been working late this week. I'll be reviewing this over the weekend.

- Added a .gitignore file so vendor folder and composer.lock file are
  not added to VCS
- Updated PlancakeEmailParser.php to use PSR-2 code style
- Added squizlabs/php_codesniffer to require-dev
- Updated tests/run_tests.php to use PSR-2 code style

Use codesniffer 2.5.1

Added a more obvious barrier
- Added clearer barriers and barriers between the different output
  options in a message
@djmattyg007 djmattyg007 merged commit f0a3a3b into djmattyg007:master Mar 9, 2016
@djmattyg007
Copy link
Owner

Sorry for the delay. I had actually reviewed this a couple of days ago, but only just now got around to merging it in and pushing it up. Thanks for your contribution!

@djmattyg007
Copy link
Owner

As an FYI, once I merge in this PR, I'll be tagging a new release as v3.0.0.

daniele-occhipinti#30

@ibrahimlawal
Copy link
Author

Awesome. Excellent work.

@djmattyg007
Copy link
Owner

I've just pushed v3.0.0!

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