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

Downgrade blueprintjs version in the web console to one with a vanilla Apache 2.0 license #7139

Merged
merged 4 commits into from
Feb 26, 2019

Conversation

vogievetsky
Copy link
Contributor

@vogievetsky vogievetsky commented Feb 25, 2019

The new console implemented by #6923 used blueprint.js v3 which has a modified Apache 2.0 license (see clause 10), (see discussion here also).
This has was discovered after the console was already implemented.
A request was raised with Apache legal to see if the modified blueprint Apache license would be acceptable to use in this project.

In the meantime, and so as not to hold up the upcoming 0.14.0 release, this PR downgrades blueprint from v3 to v1.0.1 which was licensed under a vanilla Apache license (as can be seen from the license, readme, and file headers). Blueprint v1.0.1 has a similar look and feel to Blueprint v3.

This change was made with the hope that the modified blueprint license will be approved and the code could in the future be migrated back to the latest blueprint version. If the blueprint license is not approved then it would be recommended to move to a different UI library.

Here is a list of all the major changes that were made to make this work:

  • BPv1 uses an icon font instead of SVG icons which requires copying in the assets folder
  • The components/filler file is all the shimmed components that in BPv3 were implemented as react components and in BPv1 were pure CSS components
  • Using the (React as any).PropTypes = require('prop-types') trick to slap the PropTypes back into react 16 (BPv1 expects react 15)
  • Using react-addons-css-transition-group despite deprecation warning
  • Changed all dialogs to be inline because the portals did not work in the react 15 / react 16 change
  • Changed all dialogs to be created on opening (instead of using the isOpen prop)
  • Switched to using AceEditor instead of BP's EditableText which is way better anyway.

@fjy fjy added this to the 0.14.0 milestone Feb 25, 2019
@fjy
Copy link
Contributor

fjy commented Feb 25, 2019

👍

@vogievetsky
Copy link
Contributor Author

Woops, needed to do (React as any).PropTypes = require('prop-types') for the jest tests also

Copy link
Contributor

@gianm gianm left a comment

Choose a reason for hiding this comment

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

Thanks - this approach looks good to me. I agree that going with a cleanly licensed older version is the right call for now, while we wait for the wheels of license approval on the new one to keep moving. If it ends up moving to a "disallow" then I agree with the suggestion of migrating to a different library at that time.

Copy link
Member

@clintropolis clintropolis left a comment

Choose a reason for hiding this comment

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

LGTM after CI 👍

Pulled branch and tested locally, UI functional afaict.

"@types/d3-array": "^1.2.6",
"@types/jest": "^24.0.6",
"@types/lodash.debounce": "^4.0.5",
"@types/mocha": "^5.2.6",
Copy link
Contributor

Choose a reason for hiding this comment

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

Are all of these version updates necessary, and can we use fixed package versions instead of "^"?

I think it would be simpler for review and also tracking our dependencies/licenses if we minimize version upgrades

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just upgraded to the latest versions as part of my testing. I figured since I was messing with the package versions anyway I would update things to the latest versions as I was going to be doing comprehensive end-to-end testing anyway. As for using the non-fixed versions that is not an issue as there is a package-lock file included which locks those versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

also almost all of the 'spurious' things I upgraded are devDependancies in the normal dependency list every change is needed for this PR except @types/hjson which should actually probably be in the dev list also.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved @types/hjson to dev list

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed all the package changes other than what is necessary for this PR

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool, thanks

@jon-wei jon-wei merged commit b8f7620 into apache:master Feb 26, 2019
jon-wei pushed a commit to jon-wei/druid that referenced this pull request Feb 27, 2019
…a Apache 2.0 license (apache#7139)

* revert bp

* fix tests

* move @types/hjson to dev dep

* removed all the package upgrades
clintropolis pushed a commit that referenced this pull request Mar 1, 2019
…a Apache 2.0 license (#7139) (#7141)

* revert bp

* fix tests

* move @types/hjson to dev dep

* removed all the package upgrades
@vogievetsky vogievetsky deleted the bp-revert branch September 22, 2019 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants