- 
                Notifications
    You must be signed in to change notification settings 
- Fork 4
Feature/tutorial type #127
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
Conversation
| Current coverage is 88.47% (diff: 68.18%)@@             master       #127   diff @@
==========================================
  Files            69         70     +1   
  Lines          1546       1562    +16   
  Methods         282        284     +2   
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1373       1382     +9   
- Misses          173        180     +7   
  Partials          0          0          
 | 
| hmmm should probably add some tests for this specifically I guess | 
        
          
                app/config.php
              
                Outdated
          
        
      | 'appName' => basename($_SERVER['argv'][0]), | ||
| ExerciseDispatcher::class => function (ContainerInterface $c) { | ||
| WorkshopType::class => WorkshopType::STANDARD(), | ||
| ExerciseDispatcher::class => factory(function (ContainerInterface $c) { | 
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.
any reason you added this? - it doesn't actually need it
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.
just thought it would be better to be explicit about the type by default
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.
Just asked cuz i was removing as I found them 😂
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.
oh wait... sorry that's something else
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.
how the fuck
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.
yeah will put that back
        
          
                src/Factory/MenuFactory.php
              
                Outdated
          
        
      | $exercise->getName(), | ||
| $exerciseRenderer, | ||
| $userState->completedExercise($exercise->getName()), | ||
| $workshopType == WorkshopType::TUTORIAL() && !$isCurrent && !$isComplete | 
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 I prefer the check like $workshopType->getValue() === WorkshopType:: TUTORIAL, I just can't get along with the == checks :(
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.
Also can you extract this check to a method, it's not really obvious what it's doing
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.
yeah good shout not sure why I didn't just do that anyway
| class WorkshopType extends Enum | ||
| { | ||
| const STANDARD = 1; | ||
| const TUTORIAL = 2; | 
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.
You can actually add methods here if you wanted like isTutorialMode() then you can just call that up above
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.
ahh sweet would be a lot more expressive 👍
992da88    to
    6c23e8a      
    Compare
  
    | @mikeymike can haz tests? | 
Adds new functionality for a tutorial mode forcing users of these types of workshops to complete exercises one by one.
Created types so that maybe we can extend in the future in some form, e.g. timed etc