Skip to content

ZEPPELIN-2268. Adding png and jpg support for helium module imports. - #2313

Closed
volumeint wants to merge 3 commits into
apache:masterfrom
volumeint:master
Closed

ZEPPELIN-2268. Adding png and jpg support for helium module imports.#2313
volumeint wants to merge 3 commits into
apache:masterfrom
volumeint:master

Conversation

@volumeint

Copy link
Copy Markdown
Contributor

What is this PR for?

To allow helium modules to import CSS that contains references to png or jpg images.

What type of PR is it?

Improvement

Todos

  • - Task

What is the Jira issue?

How should this be tested?

The volume-leaflet helium module [https://github.com/volumeint/helium-volume-leaflet] imports the Leaflet css file. That css file references a couple of png files. Before this change, the helium module would fail to enable, after the change it succeeds.

Screenshots (if appropriate)

Questions:

  • Does the licenses files need update? No.
  • Is there breaking changes for older versions? No.
  • Does this needs documentation? No.

@Leemoonsoo

Copy link
Copy Markdown
Member

\cc @1ambda

@1ambda

1ambda commented May 4, 2017

Copy link
Copy Markdown
Member

LGTM

however, @volumeint Could you setup travis-ci for your fork and see if it passes the test? Please take a look CI.

FYI,

  • forked repository name must be zeppelin

@volumeint

Copy link
Copy Markdown
ContributorAuthor

I configured travis-ci for my fork. Build passes, but I had to re-run two of the build jobs due to flaky tests.
https://travis-ci.org/volumeint/zeppelin

test: /\.ttf(\?\S*)?$/, loader: 'url-loader', }, {
test: /\.svg(\?\S*)?$/, loader: 'url-loader', }, {
test: /\.png(\?\S*)?$/, loader: 'url-loader', }, {
test: /\.jpg(\?\S*)?$/, loader: 'url-loader', }, {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

perhaps this is better in the future if configurable by admin?

@1ambda1ambdaMay 5, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It would be better to use webpack.config.js provided by helium developer.

For example, if a helium project includes webpack.config.js, apply it instead of using default one.

@Leemoonsoo

Copy link
Copy Markdown
Member

LGTM and merge to master if no further discussions.

@asfgitasfgit closed this in 246c3d0May 6, 2017
michelemilesi pushed a commit to icteam-spa/zeppelin that referenced this pull request May 11, 2017
### What is this PR for?
To allow helium modules to import CSS that contains references to png or jpg images.
### What type of PR is it?
Improvement
### Todos
* [ ] - Task
### What is the Jira issue?
* [ZEPPELIN-2268]
### How should this be tested?
The volume-leaflet helium module [https://github.com/volumeint/helium-volume-leaflet] imports the Leaflet css file. That css file references a couple of png files. Before this change, the helium module would fail to enable, after the change it succeeds.
### Screenshots (if appropriate)
### Questions:
* Does the licenses files need update? No.
* Is there breaking changes for older versions? No.
* Does this needs documentation? No.
Author: Thomas Grant <tgrant@volumeintegration.com>
Closesapache#2313 from volumeint/master and squashes the following commits:
672b789 [Thomas Grant] Merge branch 'master' of https://github.com/apache/zeppelinc736dbe [Thomas Grant] Merge branch 'master' of https://github.com/apache/zeppelin19500d5 [Thomas Grant] ZEPPELIN-2268. Adding png and jpg support for helium module imports.
Sign up for freeto 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

@volumeint@Leemoonsoo@1ambda@felixcheung