Skip to content
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 xblock-vectordraw to requirements. #10515

Closed
wants to merge 1 commit into from

Conversation

itsjeyd
Copy link
Contributor

@itsjeyd itsjeyd commented Nov 5, 2015

This uses the development branch for now to make it easy to set up and maintain a sandbox.

JIRA ticket: OSPR-935

@openedx-webhooks
Copy link

Thanks for the pull request, @itsjeyd! It looks like you're a member of a company that does contract work for edX. If you're doing this work as part of a paid contract with edX, you should talk to edX about who will review this pull request. If this work is not part of a paid contract with edX, then you should ensure that there is an OSPR issue to track this work in JIRA, so that we don't lose track of your pull request.

To automatically create an OSPR issue for this pull request, just visit this link: https://openedx-webhooks.herokuapp.com/github/process_pr?number=10515&repo=edx%2Fedx-platform

@itsjeyd
Copy link
Contributor Author

itsjeyd commented Nov 5, 2015

@sarina Based on the discussion you had with @smarnach about problems with the bot, I manually created an OSPR issue for this. Pinging you and @cptvitamin to let you know that we are ready to get the review process started.

@cptvitamin This is the PR that pulls in the work on Vector Drawing that I mentioned in my email - if you have comments on the existing implementation or any of the additional features we are planning, let me know!

@sarina
Copy link
Contributor

sarina commented Nov 5, 2015

Hi @itsjeyd we're looking at this now. I'm really concerned about accessibility, but also student feedback. When I hit "Check", it only tells me if one of my vectors is wrong, no feedback about the other two. There's no nudging me to the right answer. There's no show answer to show me the right answer.

@sarina
Copy link
Contributor

sarina commented Nov 5, 2015

I'm moving my comments to the xblock PR, it doesn't make sense to discuss here.

@openedx-webhooks
Copy link

Thanks for the pull request, @itsjeyd! I've created OSPR-937 to keep track of it in JIRA. JIRA is a place for product owners to prioritize feature reviews by the engineering development teams.

Feel free to add as much of the following information to the ticket:

  • supporting documentation
  • edx-code email threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will still be done via the GitHub pull request interface. As a reminder, our process documentation is here.

@openedx-webhooks openedx-webhooks added open-source-contribution PR author is not from Axim or 2U needs triage labels Nov 6, 2015
@itsjeyd itsjeyd force-pushed the tim/add-xblock-vectordraw branch from ef520cb to dce40a2 Compare November 6, 2015 10:57
@sarina
Copy link
Contributor

sarina commented Nov 6, 2015

@itsjeyd It's confusing to have this PR and open-craft/xblock-activetable#1 open (as you can see I put comments here that I meant to put on the actual xblock pr). The review is going to happen at open-craft/xblock-activetable#1, and we'll use your sandbox you have set up for review. Merging the hash to requirements is really just a quick thing once everyone's agreed on the xblock review, so I'm going to close this for now. Once we're all OK with the xblock itself, it'll be a quick PR to get the requirements updated.

@sarina sarina closed this Nov 6, 2015
@smarnach
Copy link
Contributor

smarnach commented Nov 9, 2015

@sarina Just to explain why we do this, we have some tooling that automatically deploys a sandbox when we open a PR against edx-platform, so opening a PR is the easiest way for us to deploy a sandbox. I did the same for xblock-activetable, though I didn't create an OSPR ticket.

Is this workflow confusing for you even without the OSPR ticket?

@sarina
Copy link
Contributor

sarina commented Nov 9, 2015

The confusing part about having 2 ospr tickets is that when you have to
coordinate work between multiple departments, they don't know where to
look. I'd like to have a single canonical OSPR representing review of the
XBlock, and for that OSPR to link to the pr in the xblock repo. If you want
to reopen this and not reopen the ospr ticket I think that makes most
sense. Sorry it's all confusing, I didn't realize you guys had tooling like
that.

On Mon, Nov 9, 2015 at 12:22 PM, Sven Marnach [email protected]
wrote:

@sarina https://github.com/sarina Just to explain why we do this, we
have some tooling that automatically deploys a sandbox when we open a PR
against edx-platform, so opening a PR is the easiest way for us to deploy a
sandbox. I did the same for xblock-activetable
https://github.com/edx/edx-platform/pull/10476, though I didn't create
an OSPR ticket.

Is this workflow confusing for you even without the OSPR ticket?


Reply to this email directly or view it on GitHub
https://github.com/edx/edx-platform/pull/10515#issuecomment-155131227.

@smarnach
Copy link
Contributor

@itsjeyd I've reprovisioned the sandbox, so you can upload an example. (Keep in mind that the databases for sandboxes are volatile, so keep a backup of the example somewhere.)

I don't think it's necessary to reopen this PR – its mere existence is enough for sandbox provisioning.

@itsjeyd
Copy link
Contributor Author

itsjeyd commented Nov 16, 2015

@smarnach Thanks for reprovisioning the sandbox!

@itsjeyd
Copy link
Contributor Author

itsjeyd commented Dec 10, 2015

@sarina I rebased on master and then force-pushed before reopening this PR. Apparently you're not supposed to do that if you want to be able to reopen a PR. I created a new PR here: #10932. Sorry about that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage open-source-contribution PR author is not from Axim or 2U
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants