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

Added support for phone, address and questions #1

Merged
merged 9 commits into from
Feb 27, 2016
Merged

Conversation

aclarkd
Copy link
Member

@aclarkd aclarkd commented Jan 30, 2016

@todo feedback on the register button
@todo error on missing required params (projectId, clientId, etc)
@todo make configuation more obvious

* @param Mixed String|Array
*/
public function addQuestion($path, $question, $answer) {
$answers = !is_array($answer) ? [['id' => '', 'text' => $answer]] : $answer;
Copy link
Contributor

Choose a reason for hiding this comment

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

It's best not to use boolean inversions if not necessary (i.e. remove the '!')

@tutfoo
Copy link
Contributor

tutfoo commented Feb 2, 2016

comment blocks should start with a /** instead of a /* to take advantage of PHPdoc

@tutfoo
Copy link
Contributor

tutfoo commented Feb 5, 2016

I will be taking over changes to this pull request

@tutfoo
Copy link
Contributor

tutfoo commented Feb 9, 2016

Please don't merge until squashed - if you're happy with these changes, let me know to squash first before you merge

/* These variables should only be attached to the request on the server side */
$clientId = 19;
$projectId = 6;
/**
Copy link
Contributor

Choose a reason for hiding this comment

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

Use '/*' for comment blocks that aren't phpdoc

@todo feedback on the register button
@todo error on missing required params (projectId, clientId, etc)
@todo make configuation more obvious
@tutfoo
Copy link
Contributor

tutfoo commented Feb 12, 2016

Fixed /** to /* comments in signup.php and squashed.

* primary: true
* }
*/
private $addresses;
Copy link
Contributor

Choose a reason for hiding this comment

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

Not initialized as an array, might be an issue with line 151

$lassoUid = '';
$apiKey = '';

if (empty($clientId) || empty($projectId) || empty($apiKey)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this. Error messages should give hints how to fix said error.

trb added a commit that referenced this pull request Feb 27, 2016
Added support for phone, address and questions
@trb trb merged commit 007ac19 into master Feb 27, 2016
@trb trb deleted the example-submitter branch March 1, 2016 21:58
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.

3 participants