#160 let's get rid of List.add() - Part 3 #234

super132 · 2015-05-01T03:05:26Z

For task #160, introducing Transform for eliminating the use of List.add() in RsWithHeaders. This PR is split from #183

super132 · 2015-05-01T03:18:37Z

@yegor256 , A iterable transform class Transform is introduced to replace the use of List.add() in RsWithHeaders with the conjunction use of Concat. It depends on #232 for changing the implementation of RsWithHeaders and that is why I have added a puzzle here.

super132 · 2015-05-01T03:26:17Z

@davvd , This PR is split from #183 , not sure if it needs to be reviewed by architect again. Please advise. Thanks.

davvd · 2015-05-01T09:27:40Z

@super132 Thanks for your pull request, let me find someone who can review it

davvd · 2015-05-01T09:37:24Z

@darkled please review,thanks

asaen · 2015-05-01T09:42:22Z

src/main/java/org/takes/rs/RsWithHeaders.java

@@ -52,6 +52,10 @@ public RsWithHeaders(final Response res, final CharSequence... headers) {
        this(res, Arrays.asList(headers));
    }

+    //@todo #160:DEV To implement the concatenation and


@super132 let's move this into the javadoc below

asaen · 2015-05-01T09:45:07Z

@super132 a minor comment above and there are some failed tests

super132 · 2015-05-01T12:56:41Z

@darkled , I moved the codes into the javadoc. Please have a check. Thanks.
The test cases failure was not from my code and the codes in master branch contains a PMD error.

asaen · 2015-05-01T21:38:21Z

@super132 could you please fix this PMD violation? otherwise it would be impossible to merge your changes.

super132 · 2015-05-02T03:03:17Z

@darkled, the build is fixed by merging the latest commits from master. Please have a check. Thanks.

asaen · 2015-05-02T09:04:32Z

@super132 thanks

asaen · 2015-05-02T09:04:36Z

@rultor merge

rultor · 2015-05-02T09:05:36Z

@rultor merge

@darkled Thanks for your request. @yegor256 Please confirm this.

yegor256 · 2015-05-04T23:53:09Z

@rultor merge

rultor · 2015-05-04T23:53:37Z

@rultor merge

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here

rultor · 2015-05-05T00:02:16Z

@rultor merge

@yegor256 Oops, I failed. You can see the full log here (spent 9min)

Running org.takes.facets.auth.social.PsTwitterTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 1, Time elapsed: 0 sec - in org.takes.facets.auth.social.PsTwitterTest

Results :

Tests in error: 
  RqMultipartTest.consumesHttpRequest:303 » IO Failed POST request to http://loc...

Tests run: 163, Failures: 0, Errors: 1, Skipped: 4

[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 49.652 s
[INFO] Finished at: 2015-05-05T00:01:05+00:00
[INFO] Final Memory: 24M/462M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.17:test (default-test) on project takes: There are test failures.
[ERROR] 
[ERROR] Please refer to /home/r/repo/target/surefire-reports for the individual test results.
[ERROR] -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.17:test (default-test) on project takes: There are test failures.

Please refer to /home/r/repo/target/surefire-reports for the individual test results.
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:212)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:153)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:145)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:116)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:80)
    at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build(SingleThreadedBuilder.java:51)
    at org.apache.maven.lifecycle.internal.LifecycleStarter.execute(LifecycleStarter.java:120)
    at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:347)
    at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:154)
    at org.apache.maven.cli.MavenCli.execute(MavenCli.java:584)
    at org.apache.maven.cli.MavenCli.doMain(MavenCli.java:213)
    at org.apache.maven.cli.MavenCli.main(MavenCli.java:157)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:606)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced(Launcher.java:289)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:229)
    at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:415)
    at org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:356)
Caused by: org.apache.maven.plugin.MojoFailureException: There are test failures.

Please refer to /home/r/repo/target/surefire-reports for the individual test results.
    at org.apache.maven.plugin.surefire.SurefireHelper.reportExecution(SurefireHelper.java:82)
    at org.apache.maven.plugin.surefire.SurefirePlugin.handleSummary(SurefirePlugin.java:195)
    at org.apache.maven.plugin.surefire.AbstractSurefireMojo.executeAfterPreconditionsChecked(AbstractSurefireMojo.java:861)
    at org.apache.maven.plugin.surefire.AbstractSurefireMojo.execute(AbstractSurefireMojo.java:729)
    at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:132)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:208)
    ... 19 more
[ERROR] 
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException

asaen · 2015-05-05T12:20:20Z

@davvd we're waiting for #253 here

davvd · 2015-05-05T13:24:50Z

@davvd we're waiting for #253 here

@darkled right, let's wait for #253

davvd · 2015-05-06T12:34:40Z

@davvd we're waiting for #253 here

@darkled yes, waiting for #253

super132 · 2015-05-07T08:15:09Z

@darkled , I saw pull requests can be merged now. Shall we try merging again? Thanks.

asaen · 2015-05-07T08:42:50Z

@rultor merge

rultor · 2015-05-07T08:42:57Z

@rultor merge

@darkled Thanks for your request. @yegor256 Please confirm this.

yegor256 · 2015-05-07T17:49:36Z

@rultor merge

rultor · 2015-05-07T18:05:04Z

@rultor merge

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here

rultor · 2015-05-07T18:20:46Z

@rultor merge

@yegor256 Oops, I failed. You can see the full log here (spent 16min)

INFO: "reading src/test/java/org/takes/misc/TransformTest.java..."
INFO: "reading src/test/java/org/takes/misc/package-info.java..."
INFO: "reading src/test/java/org/takes/package-info.java..."
INFO: "reading src/test/java/org/takes/rq/CapInputStreamTest.java..."
INFO: "reading src/test/java/org/takes/rq/RqCookiesTest.java..."
INFO: "reading src/test/java/org/takes/rq/RqFormTest.java..."
INFO: "reading src/test/java/org/takes/rq/RqGreedyTest.java..."
INFO: "reading src/test/java/org/takes/rq/RqHeadersTest.java..."
INFO: "reading src/test/java/org/takes/rq/RqHrefTest.java..."
INFO: "reading src/test/java/org/takes/rq/RqLengthAwareTest.java..."
INFO: "reading src/test/java/org/takes/rq/RqLiveTest.java..."
INFO: "reading src/test/java/org/takes/rq/RqMethodTest.java..."
INFO: "reading src/test/java/org/takes/rq/RqMultipartTest.java..."
INFO: "reading src/test/java/org/takes/rq/RqPrintTest.java..."
INFO: "reading src/test/java/org/takes/rq/RqSocketTest.java..."
INFO: "reading src/test/java/org/takes/rq/RqWithHeaderTest.java..."
INFO: "reading src/test/java/org/takes/rq/RqWithHeadersTest.java..."
INFO: "reading src/test/java/org/takes/rq/RqWithoutHeaderTest.java..."
INFO: "reading src/test/java/org/takes/rq/package-info.java..."
INFO: "reading src/test/java/org/takes/rs/RsGzipTest.java..."
INFO: "reading src/test/java/org/takes/rs/RsJSONTest.java..."
INFO: "reading src/test/java/org/takes/rs/RsPrintTest.java..."
INFO: "reading src/test/java/org/takes/rs/RsTextTest.java..."
INFO: "reading src/test/java/org/takes/rs/RsVelocityTest.java..."
INFO: "reading src/test/java/org/takes/rs/RsWithCookieTest.java..."
INFO: "reading src/test/java/org/takes/rs/RsWithHeaderTest.java..."
INFO: "reading src/test/java/org/takes/rs/RsWithHeadersTest.java..."
INFO: "reading src/test/java/org/takes/rs/RsWithoutHeaderTest.java..."
INFO: "reading src/test/java/org/takes/rs/RsXSLTTest.java..."
INFO: "reading src/test/java/org/takes/rs/package-info.java..."
INFO: "reading src/test/java/org/takes/rs/xe/RsXemblyTest.java..."
INFO: "reading src/test/java/org/takes/rs/xe/XeAppendTest.java..."
INFO: "reading src/test/java/org/takes/rs/xe/XeDateTest.java..."
INFO: "reading src/test/java/org/takes/rs/xe/XeLinkHomeTest.java..."
INFO: "reading src/test/java/org/takes/rs/xe/XeLinkSelfTest.java..."
INFO: "reading src/test/java/org/takes/rs/xe/XeLinkTest.java..."
INFO: "reading src/test/java/org/takes/rs/xe/XeLocalhostTest.java..."
INFO: "reading src/test/java/org/takes/rs/xe/XeSLATest.java..."
INFO: "reading src/test/java/org/takes/rs/xe/XeTransformTest.java..."
INFO: "reading src/test/java/org/takes/rs/xe/XeWhenTest.java..."
INFO: "reading src/test/java/org/takes/rs/xe/package-info.java..."
INFO: "reading src/test/java/org/takes/tk/TkClasspathTest.java..."
INFO: "reading src/test/java/org/takes/tk/TkFilesTest.java..."
INFO: "reading src/test/java/org/takes/tk/TkHTMLTest.java..."
INFO: "reading src/test/java/org/takes/tk/TkMeasuredTest.java..."
INFO: "reading src/test/java/org/takes/tk/TkRedirectTest.java..."
INFO: "reading src/test/java/org/takes/tk/TkTextTest.java..."
INFO: "reading src/test/java/org/takes/tk/TkVerboseTest.java..."
INFO: "reading src/test/java/org/takes/tk/TkVersionedTest.java..."
INFO: "reading src/test/java/org/takes/tk/TkWithHeadersTest.java..."
INFO: "reading src/test/java/org/takes/tk/package-info.java..."
INFO: "reading src/test/resources/log4j.properties..."
INFO: "reading src/www.takes.org/index.html..."
/var/lib/gems/1.9.1/gems/pdd-0.14.2/lib/pdd.rb:150:in `rules': 1 errors, see log above (PDD::Error)
    from /var/lib/gems/1.9.1/gems/pdd-0.14.2/lib/pdd.rb:105:in `xml'
    from /var/lib/gems/1.9.1/gems/pdd-0.14.2/bin/pdd:90:in `<top (required)>'
    from /usr/local/bin/pdd:23:in `load'
    from /usr/local/bin/pdd:23:in `<main>'
ERROR: "puzzle src/main/java/org/takes/rs/RsWithHeaders.java:55-60 has an estimate of 0 minutes, which is lower than 15 minutes"

asaen · 2015-05-07T20:53:04Z

src/main/java/org/takes/rs/RsWithHeaders.java

@@ -54,6 +54,12 @@ public RsWithHeaders(final Response res, final CharSequence... headers) {

    /**
     * Ctor.
+     * @todo #160:DEV To implement the concatenation and


@super132 please add an estimation here.

ERROR: "puzzle src/main/java/org/takes/rs/RsWithHeaders.java:55-60 has an estimate of 0 minutes, which is lower than 15 minutes"

super132 · 2015-05-08T01:51:06Z

@darkled , I have added the estimate here. Please have a check. Thanks.

asaen · 2015-05-08T04:51:56Z

@rultor merge

rultor · 2015-05-08T04:52:12Z

@rultor merge

@darkled Thanks for your request. @yegor256 Please confirm this.

yegor256 · 2015-05-08T05:00:42Z

@rultor merge pls

rultor · 2015-05-08T05:01:19Z

@rultor merge pls

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here

rultor · 2015-05-08T05:14:06Z

@rultor merge pls

@yegor256 Done! FYI, the full log is here (took me 13min)

davvd · 2015-05-09T10:15:09Z

@elenavolokhova please, review this ticket for compliance with our QA rules

davvd · 2015-05-09T11:16:32Z

@rultor deploy pls

rultor · 2015-05-09T11:41:20Z

@rultor deploy pls

@davvd OK, I'll try to deploy now. You can check the progress here

rultor · 2015-05-09T11:51:42Z

@rultor deploy pls

@davvd Done! FYI, the full log is here (took me 10min)

elenavolokhova · 2015-05-11T05:10:14Z

@darkled
According to our quality rules:

Comments were mostly about design problems, not cosmetic issues.

There were mostly cosmetic issues found during review. Try to pay attention to bigger design problems and the overall quality of the code.
Please confirm that this new rule is clear.

asaen · 2015-05-11T07:41:59Z

@elenavolokhova I confirm, thank you

elenavolokhova · 2015-05-11T09:06:16Z

@davvd Quality is acceptable here.

davvd · 2015-05-11T10:34:29Z

@davvd Quality is acceptable here.

@elenavolokhova thanks a lot, next time everybody should try to make it better

davvd · 2015-05-11T18:23:33Z

@darkled 10 mins was added to the account of @elenavolokhova (for QA review), in transaction 57164971... 23 mins sent to your balance (ID 57164982), many thanks! It took 167 hours and 31 mins.... review comments (c=8) added as a bonus... +23 added to your rating, at the moment it is: +2062

Introducing Transform for changing iterables in RsWithHeaders

1afddf9

super132 mentioned this pull request May 1, 2015

#160 let's get rid of List.add() #183

Closed

super132 added 2 commits May 1, 2015 11:44

Updating javadoc

84f3a37

Updating todo comments to fix check styles error

fe6a251

davvd added CR @darkled labels May 1, 2015

asaen reviewed May 1, 2015
View reviewed changes

Putting the todo comments into javadoc.

04919af

Merge remote-tracking branch 'origin/master' into iterable-transform

1073038

super132 added 2 commits May 2, 2015 11:21

Fixing a javadoc.

2a4b1d6

Fixing minor javadoc issues

8fdedf0

super132 mentioned this pull request May 2, 2015

let's get rid of List.add() #160

Closed

Updated unit test cases

73ca774

Updated puzzle comments to have more comprehensive description.

fa432a7

asaen reviewed May 7, 2015
View reviewed changes

Updated puzzle comment by adding time budget.

8153eed

rultor merged commit 8153eed into yegor256:master May 8, 2015

super132 deleted the iterable-transform branch May 8, 2015 06:02

#160 let's get rid of List.add() - Part 3 #234

#160 let's get rid of List.add() - Part 3 #234

Conversation

super132 commented May 1, 2015

super132 commented May 1, 2015

super132 commented May 1, 2015

davvd commented May 1, 2015

davvd commented May 1, 2015

asaen May 1, 2015

Choose a reason for hiding this comment

asaen commented May 1, 2015

super132 commented May 1, 2015

asaen commented May 1, 2015

super132 commented May 2, 2015

asaen commented May 2, 2015

asaen commented May 2, 2015

rultor commented May 2, 2015

yegor256 commented May 4, 2015

rultor commented May 4, 2015

rultor commented May 5, 2015

asaen commented May 5, 2015

davvd commented May 5, 2015

davvd commented May 6, 2015

super132 commented May 7, 2015

asaen commented May 7, 2015

rultor commented May 7, 2015

yegor256 commented May 7, 2015

rultor commented May 7, 2015

rultor commented May 7, 2015

asaen May 7, 2015

Choose a reason for hiding this comment

super132 commented May 8, 2015

asaen commented May 8, 2015

rultor commented May 8, 2015

yegor256 commented May 8, 2015

rultor commented May 8, 2015

rultor commented May 8, 2015

davvd commented May 9, 2015

davvd commented May 9, 2015

rultor commented May 9, 2015

rultor commented May 9, 2015

elenavolokhova commented May 11, 2015

asaen commented May 11, 2015

elenavolokhova commented May 11, 2015

davvd commented May 11, 2015

davvd commented May 11, 2015