Skip to content

[FUTURE]CLOUDSTACK-8872: Replaced all occurences of Charset.forName(UTF-8) with StringUtils.getPreferredCharset().#832

Closed
borisroman wants to merge 1 commit into
apache:masterfrom
borisroman:charset
Closed

[FUTURE]CLOUDSTACK-8872: Replaced all occurences of Charset.forName(UTF-8) with StringUtils.getPreferredCharset().#832
borisroman wants to merge 1 commit into
apache:masterfrom
borisroman:charset

Conversation

@borisroman
Copy link
Copy Markdown
Contributor

@borisroman borisroman commented Sep 15, 2015

This PR replaces all occurrences of Charset.forName("UTF-8") with StringUtils.getPreferredCharset().

This to ensure consistent encoding across all ACS components, even when UTF-8 is not available.

@borisroman borisroman force-pushed the charset branch 3 times, most recently from e0dec7e to efa73c1 Compare September 15, 2015 15:26
@asfbot
Copy link
Copy Markdown

asfbot commented Sep 15, 2015

cloudstack-pull-rats #607 SUCCESS
This pull request looks good

@asfbot
Copy link
Copy Markdown

asfbot commented Sep 15, 2015

cloudstack-pull-rats #609 SUCCESS
This pull request looks good

@asfbot
Copy link
Copy Markdown

asfbot commented Sep 15, 2015

cloudstack-pull-rats #610 SUCCESS
This pull request looks good

@asfbot
Copy link
Copy Markdown

asfbot commented Sep 15, 2015

cloudstack-pull-rats #613 SUCCESS
This pull request looks good

@asfbot
Copy link
Copy Markdown

asfbot commented Sep 15, 2015

cloudstack-pull-analysis #542 FAILURE
Looks like there's a problem with this pull request

@asfbot
Copy link
Copy Markdown

asfbot commented Sep 15, 2015

cloudstack-pull-analysis #544 FAILURE
Looks like there's a problem with this pull request

@asfbot
Copy link
Copy Markdown

asfbot commented Sep 15, 2015

cloudstack-pull-analysis #545 FAILURE
Looks like there's a problem with this pull request

@asfbot
Copy link
Copy Markdown

asfbot commented Sep 15, 2015

cloudstack-pull-analysis #547 FAILURE
Looks like there's a problem with this pull request

@wilderrodrigues
Copy link
Copy Markdown
Contributor

wilderrodrigues commented Sep 16, 2015

Hi @borisroman

Few months ago we had a thread on the list to discuss which PR flow we should have in place in order to increase ACS quality. The idea was:

  • Each PR should contain:
    • Jira ticket
    • Proper description of the change: what it fixes / what is the improvement
    • Tests - either Unit or Integration

Except for the unit test part, which is already being covered by the StringUtilsTest class, the other 2 points are missing in your PR.

Could you please create a ticket and edit the PR description?

Cheers,
Wilder

@borisroman borisroman changed the title Replaced all occurences of Charset.forName(UTF-8) with StringUtils.getPreferredCharset(). CLOUDSTACK-8872: Replaced all occurences of Charset.forName(UTF-8) with StringUtils.getPreferredCharset(). Sep 16, 2015
@borisroman
Copy link
Copy Markdown
Contributor Author

borisroman commented Sep 16, 2015

Hi @wilderrodrigues

Thank you for your extensive comment. I've read through the thread you mentioned and added the required Jira ticket and PR description.

Thanks!

@wilderrodrigues
Copy link
Copy Markdown
Contributor

wilderrodrigues commented Sep 17, 2015

Thanks, @borisroman ! :)

I will have a look at your PR again later today.

Cheers,
Wilder

@borisroman borisroman changed the title CLOUDSTACK-8872: Replaced all occurences of Charset.forName(UTF-8) with StringUtils.getPreferredCharset(). [FUTURE]CLOUDSTACK-8872: Replaced all occurences of Charset.forName(UTF-8) with StringUtils.getPreferredCharset(). Sep 17, 2015
@asfbot
Copy link
Copy Markdown

asfbot commented Sep 27, 2015

cloudstack-pull-rats #748 SUCCESS
This pull request looks good

@asfbot
Copy link
Copy Markdown

asfbot commented Sep 27, 2015

cloudstack-pull-analysis #697 ABORTED

@remibergsma
Copy link
Copy Markdown
Contributor

remibergsma commented Oct 29, 2015

@borisroman Can you force push this PR please and rebase it with current master? Then we can run some tests! Pinging @dahn to look at the code.

@borisroman
Copy link
Copy Markdown
Contributor Author

borisroman commented Oct 29, 2015

@remibergsma @wilderrodrigues @DaanHoogland Done.

@borisroman borisroman closed this Jan 20, 2016
@borisroman
Copy link
Copy Markdown
Contributor Author

borisroman commented Jan 20, 2016

Most of them have already been resolved.

@borisroman borisroman deleted the charset branch January 20, 2016 12:09
yadvr pushed a commit that referenced this pull request Jan 20, 2021
@davidjumani @yadvr
Fixing cks instance view (#832)
4f46f3d 
* Fixing instance view

* Chaning from ip to ssh port

* Fixing html tags in text

* Adding messages to kube actions

* Removing redundant code

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
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

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@borisroman @asfbot @wilderrodrigues @remibergsma