Skip to content

Fix JavaScriptConfig.#3062

Merged
fjy merged 1 commit intoapache:masterfrom
gianm:fixjs
Jun 3, 2016
Merged

Fix JavaScriptConfig.#3062
fjy merged 1 commit intoapache:masterfrom
gianm:fixjs

Conversation

@gianm
Copy link
Copy Markdown
Contributor

@gianm gianm commented Jun 2, 2016 •
edited
Loading

Was missing a @JsonProperty annotation needed by the json configurator, also the docs had a typo.

@gianm gianm added the Bug label Jun 2, 2016
@gianm gianm added this to the 0.9.1 milestone Jun 2, 2016
@drcrallen
Copy link
Copy Markdown
Contributor

drcrallen commented Jun 2, 2016

Can you add tests for this also?

@fjy
Copy link
Copy Markdown
Contributor

fjy commented Jun 2, 2016

👍

@drcrallen
Copy link
Copy Markdown
Contributor

drcrallen commented Jun 2, 2016

io.druid.client.cache.CacheConfigTest has an example of testing property injection

@gianm
Copy link
Copy Markdown
Contributor Author

gianm commented Jun 2, 2016

@drcrallen yeah, although since this class is in druid-api that'll require a bit of moving classes around and copy/paste.

@gianm
Copy link
Copy Markdown
Contributor Author

gianm commented Jun 2, 2016

but, yeah, that sounds good.

@gianm
Copy link
Copy Markdown
Contributor Author

gianm commented Jun 2, 2016

@drcrallen added test

drcrallen
drcrallen reviewed Jun 2, 2016
View reviewed changes
Comment thread api/src/main/java/io/druid/guice/JavaScriptConfigModule.java Outdated
Copy link
Copy Markdown
Contributor

@drcrallen drcrallen Jun 2, 2016

Choose a reason for hiding this comment

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

IMHO its ok to have the test stay in druid-server.

Copy link
Copy Markdown
Contributor Author

@gianm gianm Jun 2, 2016

Choose a reason for hiding this comment

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

ha, okay, I'll move it back then.

@gianm
Copy link
Copy Markdown
Contributor Author

gianm commented Jun 2, 2016

@drcrallen moved the guice module and test to druid-server

@drcrallen
Copy link
Copy Markdown
Contributor

drcrallen commented Jun 2, 2016

👍

@gianm
Copy link
Copy Markdown
Contributor Author

gianm commented Jun 3, 2016 •
edited
Loading

stalled after running QueryGranularityTest

https://travis-ci.org/druid-io/druid/jobs/134885527

Running io.druid.granularity.QueryGranularityTest
Tests run: 31, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 16.786 sec - in io.druid.granularity.QueryGranularityTest
No output has been received in the last 10 minutes, this potentially indicates a stalled build or something wrong with the build itself.
The build has been terminated

@gianm gianm closed this Jun 3, 2016
@gianm gianm reopened this Jun 3, 2016
@gianm gianm mentioned this pull request Jun 3, 2016
Closed
@gianm
Copy link
Copy Markdown
Contributor Author

gianm commented Jun 3, 2016 •
edited
Loading

Failed again on the #3067 thing

https://travis-ci.org/druid-io/druid/jobs/134920880

@gianm gianm closed this Jun 3, 2016
@gianm gianm reopened this Jun 3, 2016
@fjy fjy closed this Jun 3, 2016
@fjy fjy reopened this Jun 3, 2016
@nishantmonu51
Copy link
Copy Markdown
Member

nishantmonu51 commented Jun 3, 2016

👍, after travis

@fjy fjy merged commit 2db5f49 into apache:master Jun 3, 2016
@gianm gianm mentioned this pull request Jun 3, 2016
Merged
@gianm gianm modified the milestones: 0.9.2, 0.9.1 Sep 23, 2016
@gianm gianm modified the milestones: 0.9.1, 0.9.2 Sep 23, 2016
@gianm gianm deleted the fixjs branch September 23, 2022 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Bug

Projects

None yet

Milestone

0.9.1

Development

Successfully merging this pull request may close these issues.

4 participants

@gianm @drcrallen @fjy @nishantmonu51