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.

Sign up for GitHub

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

Jump to bottom

Add toString() to PlaceRequest. #284

Closed
wants to merge 5 commits into from
Closed

Add toString() to PlaceRequest. #284

wants to merge 5 commits into from

Conversation

mwuertinger
Copy link

Adding a toString() method to PlaceRequest simplifies debugging.

@mwuertinger
Add toString() to PlaceRequest.
9a7505f 
Adding a toString() method to PlaceRequest simplifies debugging.
branflake2267
branflake2267 reviewed Jul 8, 2013
View reviewed changes
gwtp-core/gwtp-mvp-client/src/test/java/com/gwtplatform/mvp/client/proxy/PlaceRequestTest.java
@@ -75,4 +75,17 @@ public void shouldBuildRequestWithParameterMap() {
assertEquals("value2", request.getParameter("name2", ""));
assertEquals("value3", request.getParameter("name3", ""));
}

@Test
public void testToString() {
Copy link
Contributor

Choose a reason for hiding this comment

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

extra bonus, nice :)

@branflake2267
Copy link
Contributor

LGTM.

branflake2267
branflake2267 reviewed Jul 8, 2013
View reviewed changes
gwtp-core/gwtp-mvp-client/src/main/java/com/gwtplatform/mvp/client/proxy/PlaceRequest.java
@@ -166,6 +166,17 @@ public int hashCode() {
return 11 * (nameToken.hashCode() + (params == null ? 0 : params.hashCode()));
}

@Override
public String toString() {
StringBuilder builder = new StringBuilder();
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't decide if StringBuilder is overkill. Seems like String concatenation is enough. What do you think @christiangoudreau?

Copy link
Member

Choose a reason for hiding this comment

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

It is overkill, the only place where the Compiler isn't smart enough to do replace concatenation with string builders is within loops.

@christiangoudreau
Copy link
Member

Format should be 4 spaces

@branflake2267
Copy link
Contributor

Is the teamcity not running the pulls?

@christiangoudreau
Copy link
Member

may be down

@mwuertinger
Add toString() to PlaceRequest.
f8e5ba3 
Adding a toString() method to PlaceRequest simplifies debugging.
@mwuertinger
Copy link
Author

It's been a while, what's the status on this?
I'm asking because we just ran into this issue again ;)

@Chris-V
Copy link
Member

Chris-V commented Jul 30, 2013

Your build is failing. Make sure you can successfully run mvn clean package.
At a glance from the CI server:

[04:31:23][com.gwtplatform:gwtp-mvp-client] /home/arcbees/TeamCity/buildAgent/work/c28b7c042e9b01b8/gwtp-core/gwtp-mvp-client/src/main/java/com/gwtplatform/mvp/client/proxy/PlaceRequest.java:187:1: File contains tab characters (this is the first instance).
[04:31:23][com.gwtplatform:gwtp-mvp-client] /home/arcbees/TeamCity/buildAgent/work/c28b7c042e9b01b8/gwtp-core/gwtp-mvp-client/src/test/java/com/gwtplatform/mvp/client/proxy/PlaceRequestTest.java:77: Line matches the illegal pattern 'trailing whitespace'.
[04:31:23][com.gwtplatform:gwtp-mvp-client] /home/arcbees/TeamCity/buildAgent/work/c28b7c042e9b01b8/gwtp-core/gwtp-mvp-client/src/test/java/com/gwtplatform/mvp/client/proxy/PlaceRequestTest.java:83: Line matches the illegal pattern 'trailing whitespace'.
[04:31:23][com.gwtplatform:gwtp-mvp-client] /home/arcbees/TeamCity/buildAgent/work/c28b7c042e9b01b8/gwtp-core/gwtp-mvp-client/src/test/java/com/gwtplatform/mvp/client/proxy/PlaceRequestTest.java:89:9: File contains tab characters (this is the first instance).

@christiangoudreau
Copy link
Member

Please fix build issues.

@mwuertinger
Add toString() to PlaceRequest.
d7cddbb 
Adding a toString() method to PlaceRequest simplifies debugging.
@mwuertinger
Add toString() to PlaceRequest.
b938225 
Adding a toString() method to PlaceRequest simplifies debugging.
@mwuertinger
Add toString() to PlaceRequest.
7336c39 
Adding a toString() method to PlaceRequest simplifies debugging.
@branflake2267
Copy link
Contributor

Need any help to get it to pass?

@mwuertinger
Copy link
Author

That would be great :)
I'm not sure, but it looks to me that the build even fails without my changes. Can I somehow rebase my changes on master without creating a new pull request?

@branflake2267
Copy link
Contributor

What IDE are you using?

@christiangoudreau
Copy link
Member

No, you must make sure that there's no conflict and that the build is passing. You must first merge changes from master into your branch/fork.

Also, our opensource CI will be changed soon, right now its resource are limited and something the build fail for no good reason.

@Chris-V Chris-V mentioned this pull request Mar 7, 2014
Merged
@Chris-V
Copy link
Member

Chris-V commented Mar 7, 2014

Merged from #448

@Chris-V Chris-V closed this Mar 7, 2014
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
@mwuertinger @branflake2267 @christiangoudreau @Chris-V