-
Notifications
You must be signed in to change notification settings - Fork 11
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
fix a bug I introduced into 0.5.0 + added unittests & updated docs #6
Conversation
Oops I broke all the add*() methods such that they only set the first iteration addEmail('user1@test.com'); addEmail('user2@test.com'); // should not be overwriting the above!
tests set() for all standard properties and tests all add*() and set*() methods. tests still need to be created to test that get() / fetch() return properly escaped and encoded strings
completed the testPropertyGroups and testParameters tests
now uses the set() method
not sure where this is seen/used... updated examples to use the new set() method
I will take a look tomorrow – thanks for helping! :) |
} | ||
|
||
/** | ||
* @deprecated | ||
* @see self::set() | ||
*/ | ||
public function addOrganization($val) { | ||
public function addOrganization($val,$append=true) { | ||
if ( $append && !empty($this->value['ORG'][0]) ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spaces / coding style here - Run PHPCS over the code.
after running phpCS
after running phpCS
after running phpCS
after running phpCS
I'm still not gitHub proficient, but I've updated 4 files to make phpCS mostly happy (at least happy concerning any edits I've made). Let me know if the updates aren't part of my open pull request. Or if I need to to do anything else. |
I was wondering about that... the "new" in the depreciate methods was something I was going to add until I looked at this fix. Going to try it out. |
RTBC |
@generalredneck @bkdotcom What should we do here? |
I believe @generalredneck is is saying all is well. and this fix is Ready to be Tested By the Community |
@generalredneck @bkdotcom Can either of you help my merging master into this? |
# Conflicts: # File/IMC/Build.php # tests/AllTests.php
phpunit updates & whitespace added .gitattributes: "* text eol=lf" test was failing due to line-ending conversion
2.5 years later... |
7.5 years after the issue was created, I ask if this should still be open |
@generalredneck Might depend on whether this library is abandoned? I'd love to see this committed. |
This diff is huge... are there any actual code changes rather than CS? |
0.6.1 tagged for release |
whew - phpUnit's documentation is sorely lacking in the how to install and run your first test department.
I apologize for the delay.
Most importantly, I discovered that I broke all the add*() methods.
addEmail('user@domain.com');
addEmail('foo@bar.com'); // oops, this is now overwriting the 1st, rather than adding an iteration
I have updated Build/Vcard.php to fix this.
I also need to add a file to the repository (a small image to the tests dir) for testing set('PHOTO') and/or set('LOGO').
apparently this can't be done via the web interface, I will need to add from home.