Skip to content

Make Repo.create() use new RepoCreate class for settings - #1095

Merged
rultor merged 2 commits into
jcabi:masterfrom
cvrebert:friendlier-repo-create
Jun 1, 2015
Merged

rultor merged 2 commits into
jcabi:masterfrom
cvrebert:friendlier-repo-create

Conversation

@cvrebert

Copy link
Copy Markdown
Contributor

Fixes #1094 by adding a new parameter class (NewRepoSettings) and changing Repo.create() to accept an instance of this class as its argument rather than a raw JsonObject.

This requires updating a lot of test code since testcases create lots of temporary repos.

@dmarkov

dmarkov commented May 27, 2015

Copy link
Copy Markdown

@cvrebert Let me find a reviewer for this pull request, thanks for submitting it

@dmarkov

dmarkov commented May 27, 2015

Copy link
Copy Markdown

@pinaf please review, thanks

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@cvrebert I'd rather have it named RepoCreate. In the future other actions encoded in json could be RepoDelete, etc

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@pinaf

pinaf commented May 27, 2015

Copy link
Copy Markdown
Contributor

@cvrebert even though this PR is rather long, I really like what it does to the code :)

@pinaf

pinaf commented May 27, 2015

Copy link
Copy Markdown
Contributor

@cvrebert 10 comments above.

@cvrebert

Copy link
Copy Markdown
Contributor Author

@pinaf Responded to all your comments.

@pinaf

pinaf commented May 28, 2015

Copy link
Copy Markdown
Contributor

@cvrebert thanks. 1 comment above.

@cvrebert

Copy link
Copy Markdown
Contributor Author

@pinaf Done. Addressed that comment.

@pinaf

pinaf commented May 28, 2015

Copy link
Copy Markdown
Contributor

@rultor merge
@cvrebert thank you

@rultor

rultor commented May 28, 2015

Copy link
Copy Markdown
Contributor

@rultor merge
@cvrebert thank you

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

@cvrebert cvrebert changed the title Make Repo.create() use NewRepoSettings class Make Repo.create() use new RepoCreate class for settings May 28, 2015
@cvrebert

Copy link
Copy Markdown
Contributor Author

@yegor256 Please try merging? Hopefully this will have better luck than the other PR.

@cvrebert

Copy link
Copy Markdown
Contributor Author

@yegor256 Please review+merge this.

@yegor256

Copy link
Copy Markdown
Member

@rultor try to merge again

@rultor

rultor commented May 29, 2015

Copy link
Copy Markdown
Contributor

@rultor try to merge again

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

@rultor

rultor commented May 29, 2015

Copy link
Copy Markdown
Contributor

@rultor try to merge again

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

[INFO] [INFO] Finished at: 2015-05-29T20:07:59+00:00
[INFO] [INFO] Final Memory: 18M/276M
[INFO] [INFO] ------------------------------------------------------------------------
[INFO] [WARNING] The requested profile "qulice" could not be activated because it does not exist.
[INFO] ..SUCCESS (7.1 s)
[INFO] -------------------------------------------------
[INFO] Build Summary:
[INFO]   Passed: 2, Failed: 0, Errors: 0, Skipped: 0
[INFO] -------------------------------------------------
[INFO] 
[INFO] --- maven-failsafe-plugin:2.17:verify (default) @ jcabi-github ---
[INFO] Failsafe report directory: /home/r/repo/target/failsafe-reports
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 01:40 min
[INFO] Finished at: 2015-05-29T20:07:59+00:00
[INFO] Final Memory: 46M/462M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-failsafe-plugin:2.17:verify (default) on project jcabi-github: There are test failures.
[ERROR] 
[ERROR] Please refer to /home/r/repo/target/failsafe-reports for the individual test results.
[ERROR] -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.apache.maven.plugins:maven-failsafe-plugin:2.17:verify (default) on project jcabi-github: There are test failures.

Please refer to /home/r/repo/target/failsafe-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/failsafe-reports for the individual test results.
    at org.apache.maven.plugin.surefire.SurefireHelper.reportExecution(SurefireHelper.java:82)
    at org.apache.maven.plugin.failsafe.VerifyMojo.execute(VerifyMojo.java:194)
    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

@pinaf

pinaf commented May 29, 2015

Copy link
Copy Markdown
Contributor

@rultor merge again

@pinaf

pinaf commented May 29, 2015

Copy link
Copy Markdown
Contributor

@yegor256 github error 500 - unrelated

@rultor

rultor commented May 29, 2015

Copy link
Copy Markdown
Contributor

@rultor merge again

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

@cvrebert

Copy link
Copy Markdown
Contributor Author

@yegor256 Maybe we could use RetryWire for RtMilestonesITCase since GitHub seems to be flaky on it?

@yegor256

Copy link
Copy Markdown
Member

@cvrebert yes, good idea, can you do it in this branch?

GitHub seems to be frequently giving us HTTP 500 error responses in this testcase.
@cvrebert

Copy link
Copy Markdown
Contributor Author

@yegor256 Done. Please retry merge.

@cvrebert

Copy link
Copy Markdown
Contributor Author

@yegor256 Please merge.

@pinaf

pinaf commented May 31, 2015

Copy link
Copy Markdown
Contributor

@rultor merge

@rultor

rultor commented May 31, 2015

Copy link
Copy Markdown
Contributor

@rultor merge

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

@yegor256

yegor256 commented Jun 1, 2015

Copy link
Copy Markdown
Member

@rultor merge

@rultor

rultor commented Jun 1, 2015

Copy link
Copy Markdown
Contributor

@rultor merge

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

@rultor
rultor merged commit 8816c6c into jcabi:master Jun 1, 2015
@rultor

rultor commented Jun 1, 2015

Copy link
Copy Markdown
Contributor

@rultor merge

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

@cvrebert
cvrebert deleted the friendlier-repo-create branch June 1, 2015 19:35
@dmarkov

dmarkov commented Jun 4, 2015

Copy link
Copy Markdown

@pinaf 22 mins added to your account (payment number AP-81V683464X307641P), many thanks for your contribution! 141 hours and 2 mins spent here.. you're getting extra minutes here (c=7). +22 to your rating, your total score is +7744

@dmarkov

dmarkov commented Jun 4, 2015

Copy link
Copy Markdown

@rultor deploy now pls

@rultor

rultor commented Jun 4, 2015

Copy link
Copy Markdown
Contributor

@rultor deploy now pls

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

@rultor

rultor commented Jun 4, 2015

Copy link
Copy Markdown
Contributor

@rultor deploy now pls

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

@dmarkov

dmarkov commented Jun 4, 2015

Copy link
Copy Markdown

@cvrebert there is a puzzle in this code 1095-deaa3e41/#1108, we'll resolve it later

@0pdd

0pdd commented Jul 5, 2022

Copy link
Copy Markdown

@cvrebert the puzzle #unknown is still not solved.

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.

Repos.create() shouldn't require usage of raw JSON

6 participants