Adding identity samples #458
Conversation
|
The Travis build failed on the PSR-2 check. You can make the necessary fixes by running the following from the project's root directory: $ vendor/bin/php-cs-fixer fix --level psr2 . |
|
|
||
| // Create user | ||
| $user = $service->createUser(array( | ||
| 'username' => '{username}', // replace username |
There was a problem hiding this comment.
'{username}' should be replaced by something like getenv('RS_IDENTITY_USER_NAME') so the user needs only use one mechanism (environment variables) to pass in information needed to execute these scripts.
There was a problem hiding this comment.
I'm actually not a big fan of asking users to set a whole bunch of environment variables. For me, that's more work than tweaking an inline variable. Plus on DRC we use inline placeholders rather than env vars, so there's added inconsistency if we do something different.
There was a problem hiding this comment.
I'm fine with using inline placeholders instead of environment variables but three considerations come to mind:
- There are several existing samples (in the
samples/directory) that would need updating, - I would rather go inline placeholders all the way than having some values specified via environment variables and others via inline placeholdes,
- Will inline placeholders work for @maxlinc's scripts?
There was a problem hiding this comment.
Okay, I'm fine with that. That's a good point about @maxlinc's scripts, though - I'll ping Max and ask what his requirements are to make it work. If inline placeholders are fine, I'll go ahead and change all the other samples and submit in another PR.
There was a problem hiding this comment.
If inline placeholders are fine, I'll go ahead and change all the other samples and submit in another PR.
@jamiehannaford I've created #467 to track progress with changing all the other code samples so we are out of our current inconsistent state (some samples use getenv(...) while others use {...}).
|
@ycombinator This is ready for another review too 😃 |
Partially addresses #422