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

Changed @NotNull to Validate.notNull() in Aether.jara #80

Merged
merged 2 commits into from
Feb 16, 2016
Merged

Changed @NotNull to Validate.notNull() in Aether.jara #80

merged 2 commits into from
Feb 16, 2016

Conversation

amihaiemil
Copy link
Member

This PR is for issue #78 . Build under openjdk6 (and oraclejdk6) fails when javax.validation.constraints.@NotNull is used for validation, so I used org.apache.commons.lang3.Validate.notNull() instead.

@amihaiemil amihaiemil mentioned this pull request Feb 14, 2016
Closed
@dmarkov
Copy link

dmarkov commented Feb 15, 2016

@amihaiemil Many thanks for the PR, let me find a reviewer for it

@dmarkov
Copy link

dmarkov commented Feb 15, 2016

@pinaf please review this PR

@pinaf
Copy link
Contributor

pinaf commented Feb 16, 2016

@amihaiemil looks good, just not sure whether we should even have these validations since they were removed in another project. @yegor256 please advise on whether these validations should be removed or whether this solution is fine.

@yegor256
Copy link
Member

@amihaiemil let's just remove them at all. see http://www.yegor256.com/2016/01/26/defensive-programming.html

@amihaiemil
Copy link
Member Author

@pinaf Done. Scope of the validation-api dependency is set to <scope>test</scope> as for the others, since the check complains of unused dependencies in the final build (validation-api jar comes from jcabi-parent).

@pinaf
Copy link
Contributor

pinaf commented Feb 16, 2016

@amihaiemil I see. Can't we add <exclusions><exclusion>...</exclusion></exclusions> to the jcabi-parent dependency?

@amihaiemil
Copy link
Member Author

@pinaf I think that should also do it. I will try as soon as possible, later today.

@amihaiemil
Copy link
Member Author

@pinaf I tried but you cannot exclude dependencies that are inherited from the pom parent. The <exclusions> child is not even accepted by a <parent> element, only by a <dependency> element. From what I read online (http://stackoverflow.com/questions/7898446/how-to-exclude-a-dependency-from-parents-project-in-maven) people also do all sorts of "hacks" to achieve this. I say we stick with this <scope>test</scope> since it's done like that in all the other projects.

@pinaf
Copy link
Contributor

pinaf commented Feb 16, 2016

@amihaiemil agreed, thanks.

@pinaf
Copy link
Contributor

pinaf commented Feb 16, 2016

@rultor merge

@rultor
Copy link
Contributor

rultor commented Feb 16, 2016

@rultor merge

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

@yegor256
Copy link
Member

@rultor merge

@rultor
Copy link
Contributor

rultor commented Feb 16, 2016

@rultor merge

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

@rultor rultor merged commit e3a29e8 into jcabi:master Feb 16, 2016
@rultor
Copy link
Contributor

rultor commented Feb 16, 2016

@rultor merge

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

@dmarkov
Copy link

dmarkov commented Feb 22, 2016

@pinaf 19 mins added to your account (payment number AP-8V970013566267729), many thanks for your contribution! 48 hours and 37 mins spent here.

you're getting extra minutes here (c=4)

+19 to your rating, your total score is +6695

@dmarkov
Copy link

dmarkov commented Feb 22, 2016

@rultor deploy now pls

@rultor
Copy link
Contributor

rultor commented Feb 22, 2016

@rultor deploy now pls

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

@rultor
Copy link
Contributor

rultor commented Feb 22, 2016

@rultor deploy now pls

@amihaiemil @dmarkov Oops, I failed. You can see the full log here (spent 3min)

"

Tests in error: 
  AetherTest.findsAndLoadsArtifacts:117 » DependencyResolution failed to load 'c...

Tests run: 16, Failures: 1, Errors: 1, Skipped: 0

[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 26.722 s
[INFO] Finished at: 2016-02-22T07:19:38+00:00
[INFO] Final Memory: 32M/396M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.17:test (default-test) on project jcabi-aether: 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 jcabi-aether: 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:128)
    at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:307)
    at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:193)
    at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:106)
    at org.apache.maven.cli.MavenCli.execute(MavenCli.java:863)
    at org.apache.maven.cli.MavenCli.doMain(MavenCli.java:288)
    at org.apache.maven.cli.MavenCli.main(MavenCli.java:199)
    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:134)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:207)
    ... 20 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
container 613fb5f32e22a23eb3691cf86aaf6c372b562a1a45c4e84761a90c6b9ebeea15 is dead
Mon Feb 22 07:19:45 UTC 2016

@amihaiemil
Copy link
Member Author

@dmarkov can you let rultor try to deploy again?

@dmarkov
Copy link

dmarkov commented Feb 26, 2016

@dmarkov can you let rultor try to deploy again?

@amihaiemil ask project architect. I'm only deploying once

@amihaiemil
Copy link
Member Author

@yegor256 This will not be deployed successfully until issue #82 is solved.

On other topic:

What is the complete workflow? In general, does the PR need to be deployed until the ticket is closed? I'm a bit confused since I have 4 or 5 other open tickets which had their PR merged and closed days ago .

@yegor256
Copy link
Member

@amihaiemil deployment is required if you are solving and removing a puzzle

@amihaiemil amihaiemil mentioned this pull request Apr 15, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@pinaf pinaf

Labels
@pinaf
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@amihaiemil @dmarkov @pinaf @yegor256 @rultor