Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

added constant for magic code#1597

Closed
vcharmcaster wants to merge 3 commits intoapache:masterapache/cloudstack:masterfrom
vcharmcaster:masterCopy head branch name to clipboard
Closed

added constant for magic code#1597
vcharmcaster wants to merge 3 commits intoapache:masterapache/cloudstack:masterfrom
vcharmcaster:masterCopy head branch name to clipboard

Conversation

@vcharmcaster
Copy link

@vcharmcaster vcharmcaster commented Jun 25, 2016

"=" was used in lots of space extracted it as constant

@vcharmcaster
added constant for magic code
bddeafb 
"=" was used in lots of space extracted it as constant
@vcharmcaster
(long)0 replaced by 0L
159948c
@syed
Copy link
Contributor

syed commented Jun 28, 2016

Looks like there are some trailing space problems. Can you please fix that @vcharmcaster

/home/jenkins/jenkins-slave/workspace/cloudstack-pr-analysis/services/console-proxy/server/src/com/cloud/consoleproxy/ConsoleProxyHttpHandlerHelper.java:25: error: Line has trailing spaces.
/home/jenkins/jenkins-slave/workspace/cloudstack-pr-analysis/services/console-proxy/server/src/com/cloud/consoleproxy/ConsoleProxyHttpHandlerHelper.java:26:1: error: Line contains a tab character.
/home/jenkins/jenkins-slave/workspace/cloudstack-pr-analysis/services/console-proxy/server/src/com/cloud/consoleproxy/ConsoleProxyHttpHandlerHelper.java:27: error: Line has trailing spaces.
/home/jenkins/jenkins-slave/workspace/cloudstack-pr-analysis/services/console-proxy/server/src/com/cloud/consoleproxy/ConsoleProxyHttpHandlerHelper.java:27:1: error: Line contains a tab character.
/home/jenkins/jenkins-slave/workspace/cloudstack-pr-analysis/services/console-proxy/server/src/com/cloud/consoleproxy/ConsoleProxyHttpHandlerHelper.java:28:1: error: Line contains a tab character.
/home/jenkins/jenkins-slave/workspace/cloudstack-pr-analysis/services/console-proxy/server/src/com/cloud/consoleproxy/ConsoleProxyHttpHandlerHelper.java:29: error: Line has trailing spaces.
/home/jenkins/jenkins-slave/workspace/cloudstack-pr-analysis/services/console-proxy/server/src/com/cloud/consoleproxy/ConsoleProxyHttpHandlerHelper.java:29:1: error: Line contains a tab character.

Also, can you rebase your commits into a single commit?

Otherwise, the code LGTM 👍

@swill
Copy link
Contributor

swill commented Jun 28, 2016

Should the result of the splits be stripped of whitespace to make sure there is no extra whitespace being saved into variables?

@vcharmcaster
Copy link
Author

vcharmcaster commented Jun 28, 2016

@swill @syed updated formatting. Let me know if i missed something.

@syed
Copy link
Contributor

syed commented Jun 28, 2016

@vcharmcaster it looks like your commit still did not fix the problems. Can you check the problems I mentioned in my previous comments. There are some trailing spaces and tabs which need to be fixed.

@yadvr
Copy link
Member

yadvr commented Aug 6, 2016

@blueorangutan package

@blueorangutan
Copy link

blueorangutan commented Aug 6, 2016

@rhtyd a Trillian-Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan
Copy link

blueorangutan commented Aug 6, 2016

Packaging result: ✖centos6 ✖centos7 ✖debian repo: http://packages.shapeblue.com/cloudstack/pr/1597

@blueorangutan
Copy link

blueorangutan commented Aug 8, 2016

Packaging result: ✖centos6 ✖centos7 ✖debian repo: http://packages.shapeblue.com/cloudstack/pr/1597
Job ID #66

@yadvr
Copy link
Member

yadvr commented Nov 20, 2016

@blueorangutan package

@blueorangutan
Copy link

blueorangutan commented Nov 20, 2016

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan
Copy link

blueorangutan commented Nov 20, 2016

Packaging result: ✖centos6 ✖centos7 ✖debian. JID-212

@yadvr
Copy link
Member

yadvr commented Nov 23, 2016

@vcharmcaster can you fix the space/tab issue.

fmaximus
fmaximus reviewed May 18, 2017
View reviewed changes
services/console-proxy/server/src/com/cloud/consoleproxy/ConsoleProxyHttpHandlerHelper.java
if (paramTokens != null && paramTokens.length == 2) {
String name = param.split("=")[0];
String value = param.split("=")[1];
String name = param.split(EQUALS)[0];
Copy link
Contributor

@fmaximus fmaximus May 18, 2017

Choose a reason for hiding this comment

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

Could be also simplified to paramTokens[0]

services/console-proxy/server/src/com/cloud/consoleproxy/ConsoleProxyHttpHandlerHelper.java
String name = param.split("=")[0];
String value = param.split("=")[1];
String name = param.split(EQUALS)[0];
String value = param.split(EQUALS)[1];
Copy link
Contributor

@fmaximus fmaximus May 18, 2017

Choose a reason for hiding this comment

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

Could be also simplified to paramTokens[1]

GabrielBrascher pushed a commit to CLDIN/cloudstack that referenced this pull request Dec 10, 2018
Address reviewers on abandoned PR apache#1597.
be67766
GabrielBrascher pushed a commit to CLDIN/cloudstack that referenced this pull request Dec 10, 2018
Address reviewers from abandoned PR apache#1597.
2f97ed5
@GabrielBrascher GabrielBrascher mentioned this pull request Dec 10, 2018
Closed
5 tasks
GabrielBrascher pushed a commit to CLDIN/cloudstack that referenced this pull request Dec 10, 2018
Address reviewers from abandoned PR apache#1597.
0ddc166
GabrielBrascher pushed a commit to CLDIN/cloudstack that referenced this pull request Dec 10, 2018
Address reviewers from abandoned PR apache#1597.
ae7c764
GabrielBrascher pushed a commit to CLDIN/cloudstack that referenced this pull request Dec 10, 2018
Address reviewers from abandoned PR apache#1597.
8de4362
GabrielBrascher pushed a commit to CLDIN/cloudstack that referenced this pull request Dec 11, 2018
Address reviewers from abandoned PR apache#1597.
58a7a6a
GabrielBrascher pushed a commit to CLDIN/cloudstack that referenced this pull request Dec 11, 2018
Address reviewers from abandoned PR apache#1597.
fd3945b
@DaanHoogland
Copy link
Contributor

DaanHoogland commented Mar 6, 2020

@vcharmcaster please rebase and re-open if still relevant

@GabrielBrascher is this still relevant?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fmaximus fmaximus fmaximus left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@vcharmcaster @syed @swill @yadvr @blueorangutan @DaanHoogland @fmaximus
Morty Proxy This is a proxified and sanitized view of the page, visit original site.