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

Configurable encoding for listing text files #603

Merged
merged 1 commit into from
Mar 19, 2015

Conversation

WonderCsabo
Copy link
Contributor

Implements #601. I followed this guide to use the property. From my understanding, this property is not defined in the Maven model at all, since can be null. I do not think we can force users to define it, so i decided to use a default value (UTF-8). But i am open to any suggestions on this. Also i am wonder: should we make this read-only? In that case, users could not set it in the mojo config, only with the global property (however, in special situations they may want to use a different value for that).

@mosabua
Copy link
Member

mosabua commented Mar 6, 2015

I think read only.. yes. And default value should maybe be the platform encoding and not hardcoded to be UTF-8.. wdyt @simpligility/android-maven-plugins-core-committers

@mosabua
Copy link
Member

mosabua commented Mar 6, 2015

And I dont think we want to support a case where the encoding is different from the rest of the source code until we see someone requests that..

@WonderCsabo
Copy link
Contributor Author

WonderCsabo commented Mar 6, 2015 via email

@WonderCsabo
Copy link
Contributor Author

WonderCsabo commented Mar 6, 2015 via email

@mosabua
Copy link
Member

mosabua commented Mar 6, 2015

Good idea about use platform encoding and warn. And I still think we should do read only. Just because others do it wrong...

This makes the Mojo to use a configurable encoding
(project.build.sourceEncoding) for reading text files when publishing
listing to Play, instead of just relying to the platform default
encoding (which can differ from the text file encoding).
@WonderCsabo
Copy link
Contributor Author

I updated the PR per @mosabua request.

@WonderCsabo
Copy link
Contributor Author

Any update on this @mosabua ?

@mosabua
Copy link
Member

mosabua commented Mar 19, 2015

Sorry ... been hung up on other stuff. Let me look at this tonight,.. I want to get the rest of the open PRs merged and cut a release after updating the SDK tool libraries as well.

@mosabua
Copy link
Member

mosabua commented Mar 19, 2015

Looks good.

mosabua added a commit that referenced this pull request Mar 19, 2015
Configurable encoding for listing text files
@mosabua mosabua merged commit 6dbe9a3 into simpligility:master Mar 19, 2015
@WonderCsabo WonderCsabo deleted the 601_publishListingEncoding branch March 19, 2015 19:43
@WonderCsabo
Copy link
Contributor Author

No problem! Thanks for reviewing and merging.

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.

2 participants