Skip to content

Conversation

@martin-henz
Copy link
Member

I suggest we merge this feature into master, so that we can get as much feedback as we can.

@coveralls
Copy link

coveralls commented Mar 1, 2019

Pull Request Test Coverage Report for Build 1201

  • 4 of 12 (33.33%) changed or added relevant lines in 6 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.03%) to 7.851%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/actions/workspaces.ts 1 2 50.0%
src/reducers/workspaces.ts 0 1 0.0%
src/components/workspace/ControlBar.tsx 0 2 0.0%
src/components/workspace/Editor.tsx 2 4 50.0%
src/containers/PlaygroundContainer.ts 0 2 0.0%
Files with Coverage Reduction New Missed Lines %
src/components/workspace/ControlBar.tsx 1 0.0%
Totals Coverage Status
Change from base Build 1193: 0.03%
Covered Lines: 280
Relevant Lines: 2994

💛 - Coveralls

@martin-henz
Copy link
Member Author

@ccyccyccy can you take a look at the merge conflicts?

@ccyccyccy
Copy link
Contributor

The autorun functionality is only working for playground, and not working in the assessment workspace. Should I add it in before we actually merge?

Also, I don't know if we should preserve the state of autorun after the user closes the browser or should we let it default to off? Currently it remembers the user's choice and sets it when they come back.

Copy link
Member

@ning-y ning-y left a comment

Choose a reason for hiding this comment

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

LGTM.

@ning-y
Copy link
Member

ning-y commented Mar 1, 2019

The autorun functionality is only working for playground, and not working in the assessment workspace. Should I add it in before we actually merge?

It's probably for the best that this feature is only available on the playground for now, so the students can catch bugs. We want to avoid bugs in the assessment workspace as far as possible, especially when the external libs are involved. Though, admittedly, I have not tested this feature out enough to judge how stable it is.

You can track auto-run for assessments workspace as a todo in the issues, or projects (whichever the active development team prefers).

This also gives time for discussion on if auto-run should be enabled for all assessments no matter what, or if some assessments should not have the auto-run feature. For example, perhaps in a particular assessment you want the students to practice visualising and predicting the output of their program as they write it. In this case, you may want to disable the option for auto-run to encourage the student to use their brains a little more.

Also, I don't know if we should preserve the state of autorun after the user closes the browser or should we let it default to off? Currently it remembers the user's choice and sets it when they come back.

Intuitively, I think that auto-run should default to off whenever a workspace is freshly rendered.

  1. I see some space for potential bugs if every source program that passes the validity test is evaluated automatically on load.
  2. I think that auto-evaluating programs may discourage students from visualising the computational processes in their programs, since it is usually given to them live via auto-run. It is my personal opinion that this laziness should not be encouraged.

@ning-y ning-y self-requested a review March 1, 2019 18:21
@ning-y
Copy link
Member

ning-y commented Mar 1, 2019

p.s. I also recommend the active dev team adopt these specifications for good commit messages. Since the 'Squash and merge' buttons on github PRs generate the squash commit message from the PR title, this also implies good PR titles. In this case, it would be something like "Add auto-run for playground (#466)".

@ning-y ning-y removed their request for review March 1, 2019 18:25
@ccyccyccy ccyccyccy changed the title added autorun functionality (#456) Add auto-run for playground (#466) Mar 2, 2019
@ccyccyccy ccyccyccy merged commit 6d079c7 into master Mar 2, 2019
@ccyccyccy ccyccyccy deleted the auto-run-feature branch March 2, 2019 07:42
@martin-henz
Copy link
Member Author

@ningyuansg that's an interesting discussion.

You are saying that the auto-run feature may discourage students from visualizing (mentally) what happens when the program runs, and instead rely on their very eyes to do that "visualization".

My hunch is that the opposite is true for beginners: I think that providing an immediate connection between the program as it is being edited and the outcome and effects of the program will facilitate the formation of mental models. I do not have evidence for this, but I would love to get some.

I'm trying to push the Source Academy towards "immediate connection": Shortening the distance between the learner and the computational process that unfolds due to his actions. For a passionate case for "immediate connection", see Bret Victor's talk "Inventing on Principle", https://vimeo.com/36579366

I'm not 100% convinced that it will be beneficial, but I see an opportunity to move in this direction, and experiment. In the same vein, we will introduce an environment visualizer in the Source Academy, and a tool to visualize the substitution model. My objective at the moment is to develop these tools and get as much exposure as we can, in order to take a decision what to include and what not to include in the Source Academy. In the same spirit, I'm arguing for enabling the autorun-feature for missions.

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.

5 participants