Skip to content

Crowdsourced Hints - "0.2 release" - #442

Merged
fephsun merged 28 commits into
masterfrom
felix/formula-hints
Aug 27, 2013
Merged

Crowdsourced Hints - "0.2 release"#442
fephsun merged 28 commits into
masterfrom
felix/formula-hints

Conversation

@fephsun

@fephsun fephsun commented Jul 19, 2013

Copy link
Copy Markdown
Contributor

Applies a bunch of improvements to the crowdsourced hints xmodule.

  • Adds support for formula response types. To do this, the formula response has been refactored slightly, to expose some internal functionality.
  • Works with rounding tolerances specified in each response type.
  • Hint-collecting UI now displays all hints that the user saw on one view.
  • Closes numerous 500 errors.
    @rocha

@ghost ghost assigned dianakhuang Jul 19, 2013
@rocha

rocha commented Jul 22, 2013

Copy link
Copy Markdown
Contributor

Seems like a test is failing. Can you fix it?

Comment thread common/lib/capa/capa/responsetypes.py Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why not compare_answer? That way is more consistent with the naming of other methods.

@rocha rocha closed this Jul 22, 2013
@rocha rocha reopened this Jul 22, 2013
@rocha

rocha commented Jul 22, 2013

Copy link
Copy Markdown
Contributor

I closed the PR accidentally, reopening.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Probably worth holding on to a version of samples.split('@') since you seem to use it a number of times.

@rocha

rocha commented Aug 2, 2013

Copy link
Copy Markdown
Contributor

@fephsun do you think this branch is ready to be merged? if so, can you fix the tests?

@fephsun

fephsun commented Aug 2, 2013

Copy link
Copy Markdown
Contributor Author

Actually, can we roll the wizard-style UX into this, as well? I'll push that up in a bit. Just have to fix some pep/pylint errors.

@fephsun

fephsun commented Aug 2, 2013

Copy link
Copy Markdown
Contributor Author

Just added the new UX. The pep8 and pylint errors aren't in code that I touched, so I think this is good.

@fephsun

fephsun commented Aug 7, 2013

Copy link
Copy Markdown
Contributor Author

@rocha Merge now?

@rocha

rocha commented Aug 7, 2013

Copy link
Copy Markdown
Contributor

Let me take a look first. Thanks.

On Wed, Aug 7, 2013 at 9:58 AM, Felix Sun notifications@github.com wrote:

@rocha https://github.com/rocha Merge now?


Reply to this email directly or view it on GitHubhttps://github.com/edx/edx-platform/pull/442#issuecomment-22252850
.

@fephsun

fephsun commented Aug 16, 2013

Copy link
Copy Markdown
Contributor Author

@auraz @vaxXxa
This xmodule is moving under the Blades team soon, so you should take a look at it, and ask me questions about any confusing or bad code that I wrote.

@vasylnakvasiuk

Copy link
Copy Markdown
Contributor

@fephsun, ok. We will review this code at Monday? Is it ok?

@fephsun

fephsun commented Aug 16, 2013

Copy link
Copy Markdown
Contributor Author

@vaxXxa Yeah, Monday is fine. Thanks for getting back to me so quickly.

Comment thread common/lib/capa/capa/responsetypes.py Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please use {} everywhere instead of dict():

In [6]: %timeit dict()
1000000 loops, best of 3: 277 ns per loop

In [7]: %timeit {}
10000000 loops, best of 3: 44.2 ns per loop

@fephsun

fephsun commented Aug 26, 2013

Copy link
Copy Markdown
Contributor Author

@valera-rozuvan Still waiting for the js/coffeescript review. I will be around until this Wednesday. Afterwards, I think @rocha can answer questions.

@valera-rozuvan

Copy link
Copy Markdown
Contributor

@fephsun I have looked over the CoffeeScript files. They look good. Can you please look into what is causing the build to be unstable? Otherwise, good to merge 👍

@fephsun

fephsun commented Aug 27, 2013

Copy link
Copy Markdown
Contributor Author

@valera-rozuvan It's pep8/pylint violations from other peoples' commits. I'll deal with this problem today, and merge.

fephsun added a commit that referenced this pull request Aug 27, 2013
Crowdsourced Hints - "0.2 release"
@fephsun
fephsun merged commit b5e1d57 into master Aug 27, 2013
@fephsun
fephsun deleted the felix/formula-hints branch August 27, 2013 17:26
@Lyla-Fischer

Copy link
Copy Markdown

Congrats! Good luck on the post-deploy support!

chrisrossi pushed a commit to jazkarta/edx-platform that referenced this pull request Mar 31, 2014
…style

Added fix for answer status and show answer to stay in the same place
antoviaque referenced this pull request in open-craft/openedx-platform May 15, 2015
Fix typo in requirements file, making "pip install" work again
xavierchan pushed a commit to xavierchan/edx-platform-1 that referenced this pull request May 30, 2019
modify price text for program
Sujeet1379 pushed a commit to chandrudev/edx-platform that referenced this pull request Nov 17, 2022
Danyal-Faheem pushed a commit to Danyal-Faheem/edx-platform that referenced this pull request Jul 15, 2025
Signed-off-by: GitHub <noreply@github.com>
Co-authored-by: ahmed-arb <ahmed-arb@users.noreply.github.com>
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.

8 participants