Skip to content

RYA-306 Remove rya.benchmark committed gen#191

Closed
ejwhite922 wants to merge 10 commits intoapache:masterfrom
ejwhite922:RYA-306_RemoveBenchmarkGen
Closed

RYA-306 Remove rya.benchmark committed gen#191
ejwhite922 wants to merge 10 commits intoapache:masterfrom
ejwhite922:RYA-306_RemoveBenchmarkGen

Conversation

@ejwhite922
Copy link
Contributor

Description

Deleted the committed generated source code in rya.benchmark from git. It is now generated and ignored by git so when building changes to those files they don't show up as changed files in git.
Also, other projects that use generated source now append the license header to the generated files. The license header is placed in the top level Rya project under "resources" so all projects can access it and has a property in the rya parent pom called "license.header.file".

Tests

Build

Links

Jira

Checklist

  • Code Review
  • Squash Commits

People To Review

@meiercaleb
@amihalik
@jdasch

<configuration>
<!-- Place the generated source within the 'src' directory so license-maven-plugin will find it. -->
<outputDirectory>src/main/gen</outputDirectory>
<outputDirectory>src/gen/java</outputDirectory>
Copy link
Contributor

@jdasch jdasch Aug 2, 2017

Choose a reason for hiding this comment

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

Line 109 is unnecessary. Remove so code is stored in ${project.build.directory}/generated-sources per maven convention.

</configuration>
</plugin>

<plugin>
Copy link
Contributor

Choose a reason for hiding this comment

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

Plugin execution is unnecessary if code is stored in ${project.build.directory}/generated-sources.

<outputDirectory>src/gen/java</outputDirectory>
</configuration>
</plugin>

Copy link
Contributor

Choose a reason for hiding this comment

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

Plugin execution is unnecessary if code is stored in ${project.build.directory}/generated-sources.

</configuration>
</plugin>

<plugin>
Copy link
Contributor

Choose a reason for hiding this comment

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

Plugin execution is unnecessary if code is stored in ${project.build.directory}/generated-sources.

@asfgit
Copy link

asfgit commented Aug 2, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-rya-master-with-optionals-pull-requests/332/

@jdasch
Copy link
Contributor

jdasch commented Aug 2, 2017

Sorry for so many comments. The main concern I have with this is that if generated source is not stored in the ${project.build.directory} (aka target), you really can't do a true mvn clean without extra build configuration. So following the maven convention gives you a lot for free and makes your poms small.

Copy link
Contributor

@isper3at isper3at left a comment

Choose a reason for hiding this comment

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

Didn't know about Jeff's comments. that's good to know moving forward. Glad to see this issue going away

Copy link
Contributor

@amihalik amihalik left a comment

Choose a reason for hiding this comment

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

I'd like to see @jdasch comments addressed before merging.

@jdasch
Copy link
Contributor

jdasch commented Aug 4, 2017

I feel bad taking up your time with this Eric. I've got a branch from a while ago that is probably good enough. Do you want me to update it and send it out as a PR, or let you mod this PR? Happy to do either.

@ejwhite922 ejwhite922 force-pushed the RYA-306_RemoveBenchmarkGen branch from a994f25 to 69dc245 Compare August 4, 2017 18:57
@asfgit
Copy link

asfgit commented Aug 4, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-rya-master-with-optionals-pull-requests/357/

Build result: FAILURE

[...truncated 5.53 MB...][INFO] Apache Rya Spark Support ........................... SKIPPED[INFO] Apache Rya Web Projects ............................ SKIPPED[INFO] Apache Rya Web Implementation ...................... SKIPPED[INFO] ------------------------------------------------------------------------[INFO] BUILD FAILURE[INFO] ------------------------------------------------------------------------[INFO] Total time: 24:17 min[INFO] Finished at: 2017-08-04T19:23:25+00:00[INFO] Final Memory: 419M/3343M[INFO] ------------------------------------------------------------------------[ERROR] Failed to execute goal com.mycila:license-maven-plugin:2.6:format (default) on project rya.export.api: Resource ${rya.project.basedir}/resources/LICENSE_HEADER.txt not found in file system, classpath or URL: no protocol: ${rya.project.basedir}/resources/LICENSE_HEADER.txt -> [Help 1][ERROR] [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.[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[ERROR] [ERROR] After correcting the problems, you can resume the build with the command[ERROR] mvn -rf :rya.export.apichannel stoppedSetting status of 69dc245 to FAILURE with url https://builds.apache.org/job/incubator-rya-master-with-optionals-pull-requests/357/ and message: 'FAILURE 'Using context: Jenkins: clean package -Pgeoindexing

@asfgit
Copy link

asfgit commented Aug 4, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-rya-master-with-optionals-pull-requests/359/

@ejwhite922 ejwhite922 force-pushed the RYA-306_RemoveBenchmarkGen branch from f39de0e to 9fc2c2e Compare August 7, 2017 18:16
@asfgit
Copy link

asfgit commented Aug 7, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-rya-master-with-optionals-pull-requests/364/

@asfgit
Copy link

asfgit commented Aug 7, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-rya-master-with-optionals-pull-requests/363/

@asfgit
Copy link

asfgit commented Aug 7, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-rya-master-with-optionals-pull-requests/362/

@ejwhite922
Copy link
Contributor Author

I made all changes requested. Let me know if anything else is needed.

@jdasch
Copy link
Contributor

jdasch commented Aug 8, 2017

Looks good. I just made a pull request to your branch with my last couple nits.

RYA-306 Added a pluginManagement definition for the license-maven-plugin
@asfgit
Copy link

asfgit commented Aug 8, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-rya-master-with-optionals-pull-requests/366/

@asfgit asfgit closed this in 2326e2d Aug 8, 2017
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.

5 participants