Skip to content

Geowave 228#230

Merged
rfecher merged 1 commit intomasterfrom
GEOWAVE-228
Feb 13, 2015
Merged

Geowave 228#230
rfecher merged 1 commit intomasterfrom
GEOWAVE-228

Conversation

@chrisbennight
Copy link
Copy Markdown
Contributor

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.01%) to 34.86% when pulling 861ce50 on GEOWAVE-228 into cd4b712 on master.

2 similar comments
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.01%) to 34.86% when pulling 861ce50 on GEOWAVE-228 into cd4b712 on master.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.01%) to 34.86% when pulling 861ce50 on GEOWAVE-228 into cd4b712 on master.

@rfecher
Copy link
Copy Markdown
Contributor

rfecher commented Feb 12, 2015

Rather than adding a specific constant in each module of a "CHAR_SET" can you just use StringUtils.UTF8_CHAR_SET? It is put in geowave-index because that is the lowest level dependency and each of the other modules can use it.

@chrisbennight
Copy link
Copy Markdown
Contributor Author

That seems completely reasonable

@chrisbennight
Copy link
Copy Markdown
Contributor Author

Moved it all over except for the ingest plugins instances; it seems reasonable that the ingest plugins might need to declare their own input encoding (though in this case they were all UTF-8)

@rfecher
Copy link
Copy Markdown
Contributor

rfecher commented Feb 13, 2015

fair enough, I don't think it really matters, but I didn't actually see why if the ingest-plugins were using UTF-8, that they wouldn't just re-use the constant for UTF-8 already defined in our string utilities...then again, does it really matter, no

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.0%) to 34.87% when pulling c285ff4 on GEOWAVE-228 into 0494211 on master.

rfecher added a commit that referenced this pull request Feb 13, 2015
@rfecher rfecher merged commit 7ec8aca into master Feb 13, 2015
@rfecher rfecher deleted the GEOWAVE-228 branch February 13, 2015 12:27
@rfecher
Copy link
Copy Markdown
Contributor

rfecher commented Feb 13, 2015

haha, I was just waiting for travis to finish and I was going to merge it this morning with the ingest plugin charsets, you didn't have to change it but thanks

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.01%) to 34.86% when pulling 26d1fec on GEOWAVE-228 into dbb8c97 on master.

2 similar comments
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.01%) to 34.86% when pulling 26d1fec on GEOWAVE-228 into dbb8c97 on master.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.01%) to 34.86% when pulling 26d1fec on GEOWAVE-228 into dbb8c97 on master.

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.

3 participants