Removed ice.min.js but left instructions for re-installation #2157
Conversation
|
@caesar2164 I'm not really sure what this feature is. I think I'll need to check if any edx courses are using this feature. |
|
@jtauber what's the right course of action here wrt ICE? |
|
@adampalay - AFAIK we at Stanford were the only ones that ever used ICE. There are edX courses that use/d ORA, but not ICE. |
|
Just curious where the confusion comes in - looking at what appears the original developer (claimed) of ice -http://mobile.nytimes.com/blogs/open/2012/01/23/introducing-ice-writing-for-the-web-first/ the listed repo shows gnu... |
|
Exhibit A: https://github.com/NYTimes/ice/blob/master/ice-master.min.js Exhibit B: https://github.com/NYTimes/ice/blob/master/LICENSE Hence, confusion... |
|
Oh! Doh! (thanks)
|
|
@nedbat is this standard practice for open source projects that possible engage with GPL v2 software? |
|
As an observation: ice includes one MIT component, rangy.js - it's license: https://code.google.com/p/rangy/source/browse/trunk/license.txt |
|
it actually "includes or is inspired by" rangy and Squiz Viper: https://github.com/NYTimes/ice/blob/master/NOTICE I'm trying to figure out if Squiz Viper is GPLv2 or GPLv2+ |
|
Thus why we want to remove ICE… |
|
Rebased... |
|
tests still failing... |
|
@adampalay the issue is that if it's GPLv2, we can't redistribute it as part of AGPLv3 software so we're playing it safe. |
|
@jtauber yeah my second question is a conventions question--do we have a convention for including instructions for installing 3rd-party libraries, and, if not, is this how we want to do it? |
|
@adampalay ah I see. I don't we have any conventions yet. I'm not a huge fan of just commenting stuff out but I'm not sure if a configuration switch is the way we want to go either. |
|
@jtauber so it sounds like for now we should remove the code from the codebase altogether until we figure out our conventions on that front |
|
@jtauber & @adampalay - with this code update, I should have fixed the test failure. Also, I already completely removed ice.min, but only commented out the piping to connect it to the ORA code. Should I remove that as well? |
|
@jtauber & @adampalay - After some discussion, we decided to completely remove track changes from core (since ORA is being reworked anyways…) so here's a new and improved PR! |
|
👍 ❤️ ✨ 😸 👍 I love deleting code! ✨ |
❗ ✨
|
👍 ! |
|
👍 |
Removed ice.min.js completely as ORA is changing anyway
|
w00t! Merged! |
|
❤️ |
* Add menu to ga_operation for ga_analyzer openedx#2039 (openedx#2088) * add role for old course viewer openedx#2062 (openedx#2087) * add role for old course viewer openedx#2062 * Change action for biz course by BetaTester role openedx#2062 * Construction of image server openedx#2025 (openedx#2106) * cherry-pick 8c8953f * Fix file upload in IE * Construction of image server openedx#2025 * add all keywords search in Student management openedx#2029 (openedx#2034) * Fix bug for before enrollment start in ga old course viewer openedx#2062 (openedx#2125) * fix. Construction of image server openedx#2025 (openedx#2117) * Modify message and css of enrollment for Course About openedx#2130 * Add a certificate list to user's profile page. openedx#2042 (openedx#2108) * Mod UT openedx#2130 * add PDF File Construction of image server openedx#2025 (openedx#2140) * add library option, and library links to the course. openedx#2001 (openedx#2124) * Invalid StudioPermissionsService object in API to show/save xblock settings in CMS. Randomized Content Block editor did not check Studio user's permissions * add library option, and library links to the course. openedx#2001 * fix. add all keywords search in Student management openedx#2029 (openedx#2034) (openedx#2157) * second fix. Construction of image server openedx#2025 (openedx#2158) * add library option, and library links to the course. openedx#2001 (openedx#2160) * third fix. Construction of image server openedx#2163 (openedx#2164) * Add filter by category for certificates on profile page openedx#2042 (openedx#2165) * Fix bug for add library option, and library links to the course. openedx#2162 openedx#2133 (openedx#2167) * Develop/dogwood/gacco201708 (openedx#2170) * Fixed bugs openedx#2039 (openedx#2112) * Fixed csv format openedx#2039 (openedx#2127) * Change to split download if there are many display items openedx#916 (openedx#2121) * Change to split download if there are many display items openedx#916 * Fix UT * Fix Review * Fix review2
As there is some confusion as to whether ICE is MIT or GPL license, this PR is to remove it from core, with future development needed to allow themed instances of OpenEdX/edX to enable it.
@adampalay & @stephensanchez - could you guys give this a look?