-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add device experiment #79
Conversation
|
||
// Now, execute all commands in the arrays. | ||
foreach ($commandarray as $shortname => $command) { | ||
tool_abconfig_execute_command_array($command, $shortname); |
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.
I think it would be good to split out most of the logic above which figures out what to do, from the execution, so that we can unit test the first half
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.
I think this depend on how much testing we want. I've left it as is for now, but I haven't gone too deep into testing the specific logic for devices (the tests I've added so far are variations of the original tests that have been slightly modified for devices).
e0c40d7
to
661e7ef
Compare
661e7ef
to
1f0f4b7
Compare
Closes #75
Closes #74 - Rather than converting everything to _before_session and trying to resolve other issues with $SESSION and $USER being null, it's safer to only have the new experiment type use it.