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

Extract opaque_keys into a separate library #3862

Merged
merged 1 commit into from
May 29, 2014

Conversation

cpennington
Copy link
Contributor

No description provided.

@cpennington
Copy link
Contributor Author

@sarina
Copy link
Contributor

sarina commented May 27, 2014

👍 assuming tests pass

@flowerhack
Copy link
Contributor

@cpennington When running this branch locally, paver install_prereqs fails to install opaque_keys (though a direct pip install of opaque_keys works fine). I'm not sure what the problem was but it seems like it might be a concern for production (maybe?)—would @jzoldak know more? I feel that regardless of whether or not it's a blocker for merging, it'd be nice to make sure install_prereqs behaves correctly

@flowerhack
Copy link
Contributor

besides that, 🎋 🍊 ⛵

@jzoldak
Copy link
Contributor

jzoldak commented May 28, 2014

@clytwynec we need to look into this ^ (paver install_prereqs issue)

@sarina
Copy link
Contributor

sarina commented May 28, 2014

@jzoldak @clytwynec -- another thing I notice is that paver install_prereqs is run 3x times during the unit test suite. I think it would be worth making it so that does not happen (right now we're just chaining calls to run different suites of tests, each of which first calls install_prereqs).

@jzoldak
Copy link
Contributor

jzoldak commented May 28, 2014

oh, right the install prereqs not picking up would be fixed if we merge in #3487 ?

@cpennington
Copy link
Contributor Author

This PR should now include the updates from #3487

@sarina
Copy link
Contributor

sarina commented May 28, 2014

👍

@flowerhack
Copy link
Contributor

@cpennington it doesn't seem to have the updates from #3487 ? in particular github.txt still isn't included in pavelib, so I'm still seeing the install_prereqs issue

@flowerhack
Copy link
Contributor

(are we still planning to merge #3487 separately...?)

@sarina
Copy link
Contributor

sarina commented May 28, 2014

@flowerhack ? #3487 was merged...

@flowerhack
Copy link
Contributor

whups, forgot to pull. @cpennington it works fine now

@cpennington
Copy link
Contributor Author

The failed test is known to be intermittent. Merging.

cpennington added a commit that referenced this pull request May 29, 2014
Extract opaque_keys into a separate library
@cpennington cpennington merged commit 81d785d into openedx:master May 29, 2014
@cpennington cpennington deleted the external-opaque-keys branch May 29, 2014 14:21
sarina added a commit that referenced this pull request Jun 5, 2014
This reverts commit 81d785d, reversing
changes made to 502b285.

Conflicts:
	requirements/edx/github.txt
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