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

Fix/adam/add edge reqs #848

Merged
merged 2 commits into from
Sep 3, 2013
Merged

Fix/adam/add edge reqs #848

merged 2 commits into from
Sep 3, 2013

Conversation

adampalay
Copy link
Contributor

@nedbat @cpennington

To add the correct packages to edx-sandbox, should we add pyparsing explicitly to the requirements, or should we install the local dependencies in editable mode (like https://github.com/edx/edx-platform/blob/master/requirements/edx/local.txt does)?

@feanil
Copy link
Contributor

feanil commented Sep 3, 2013

@adampalay local libraries are all libraries that we wrote here at edx. I think pyparsing should be added explicitly.

@adampalay
Copy link
Contributor Author

@feanil , right, I thought one needed the "-e" in order to install libraries we've written?

-e common/lib/calc
-e common/lib/chem
-e common/lib/sandbox-packages
-e common/lib/symmath
Copy link
Contributor

Choose a reason for hiding this comment

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

We definitely don't want the -e here. We need the code copied into the sandbox.

@nedbat
Copy link
Contributor

nedbat commented Sep 3, 2013

I don't understand why we use --no-deps in production and not on dev machines, but since we do, this seems like the right fix.

👍

@cpennington
Copy link
Contributor

If calc is used outside of xmodules, then we should add that requirement to the edx requirements as well.

@adampalay
Copy link
Contributor Author

I think calc is only used in xmodules, but I think it's worth adding pyparsing to edx requirements

@cpennington
Copy link
Contributor

Sorry, I meant, if calc isn't used outside of sandbox.

@adampalay
Copy link
Contributor Author

It's used for like numericalresponse, but there's never been a problem there. Something is odd about the way we install packages

@cpennington
Copy link
Contributor

👍

adampalay added a commit that referenced this pull request Sep 3, 2013
@adampalay adampalay merged commit c7a537e into master Sep 3, 2013
@adampalay adampalay deleted the fix/adam/add-edge-reqs branch September 3, 2013 16:53
jenkins-ks pushed a commit to nttks/edx-platform that referenced this pull request Jun 30, 2016
* Implements celery task management openedx#848

* Implements personalinfo mask openedx#848

* Fix batch unregister

* Fix review

* Add auth_userprofile.meta for deletion openedx#848
jenkins-ks pushed a commit to nttks/edx-platform that referenced this pull request Jun 30, 2016
jenkins-ks pushed a commit to nttks/edx-platform that referenced this pull request Jun 30, 2016
jenkins-ks pushed a commit to nttks/edx-platform that referenced this pull request Jun 30, 2016
jfavellar90 added a commit to eduNEXT/edx-platform that referenced this pull request Aug 23, 2018
caesar2164 pushed a commit to caesar2164/edx-platform that referenced this pull request Dec 11, 2018
…alytics_styling

Fix inline analytics close button styling.
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.

4 participants