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.

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

Rpm package primary #40

Merged
merged 5 commits into from
Jul 23, 2019
Merged

Rpm package primary #40

merged 5 commits into from
Jul 23, 2019

Conversation

singhald
Copy link
Contributor

This adds support for making the of the mvn project be RPM

primary difference is that when it does the install to the repo the artifact is named project-version.rpm instead of project-version-rpm.rpm which isn't a huge thing in the grand scale but still.

I added a test to make sure that the option is working

Use by adding
true
to plugin options (version, executions, extensions)

Copy link
Owner

@ctron ctron left a comment

Choose a reason for hiding this comment

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

Thanks for the PR.

I have a few remarks, but I think in general this looks good.

Please also ensure that you add yourself to the license headers of the files you changed.

src/it/test14-primary/verify.groovy Outdated Show resolved Hide resolved
src/it/test14-primary/pom.xml Outdated Show resolved Hide resolved
@ctron
Copy link
Owner

ctron commented Jul 22, 2019

Ok, this looks good. The only thing I overlooked in the beginning: could you please add yourself to the license header of the files you changes. Thanks!

@singhald
Copy link
Contributor Author

Ok, this looks good. The only thing I overlooked in the beginning: could you please add yourself to the license header of the files you changes. Thanks!

Oops missed that in the earlier request.

think I got that now

@ctron
Copy link
Owner

ctron commented Jul 22, 2019

@singhald Again, thanks for the PR and thanks for following up on the feedback.

The only thing I would like to have in addition, and it is ok to do this in a new PR, would be a bit of documentation on how to use this. Especially the src/site/apt/example.apt.vm file needs a refresh I think.

@singhald
Copy link
Contributor Author

I went ahead and added a secondary file (example-primary.apt.vm) since that looked like how the vm spec suggests doing multiple large sections, and I figured it was easier to see the differences needed that way?

Let me know if you'd rather I collapsed it back

@ctron ctron merged commit 5fcc338 into ctron:master Jul 23, 2019
@singhald singhald deleted the rpm_package_primary branch July 23, 2019 14:55
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.

2 participants