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

[CLOUD-3269] - Don't symlink module jars if the source doesn't exist #341

Merged
merged 1 commit into from Jul 12, 2019

Conversation

luck3y
Copy link
Collaborator

@luck3y luck3y commented Jun 14, 2019

https://issues.jboss.org/browse/CLOUD-3269
Signed-off-by: Ken Wills kwills@redhat.com

Thanks for submitting your Pull Request!

Please make sure your PR meets the following requirements:

  • Pull Request title is properly formatted: [CLOUD-XYA] Subject
  • Pull Request contains link to the JIRA issue
  • Pull Request contains description of the issue
  • Pull Request does not include fixes for issues other than the main ticket
  • Attached commits represent units of work and are properly formatted
  • You have read and agreed to the Developer Certificate of Origin (DCO) (see CONTRIBUTING.md)
  • Every commit contains Signed-off-by: Your Name <yourname@example.com> - use git commit -s

@luck3y luck3y requested a review from jmtd June 14, 2019 21:26
@luck3y
Copy link
Collaborator Author

luck3y commented Jun 14, 2019

@jfdenise FYI

Copy link
Collaborator

@jmtd jmtd left a comment

Choose a reason for hiding this comment

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

LGTM - another approach would be "shopt -s nullglob" at the top of the file, which makes a glob-expansion that doesn't match anything return an empty string instead of the glob pattern

but the test is sitll useful because a directory or symlink could match the pattern too, and this catches those

@jmtd
Copy link
Collaborator

jmtd commented Jun 18, 2019

This module is described as "legacy", but, it still seems to be widely used. Is there an alternative version in jboss/container/* ? If not, should we a) move/rename this module into that heirarchy and b) add a compatibility module at this name, or c) transition all the images to the new name?

@luck3y
Copy link
Collaborator Author

luck3y commented Jun 18, 2019

@jmtd I suspect we really should move this to jboss-eap-modules, since its dealing entirely with jboss-modules layers.

I think we should merge this fix and move it later though :)

@luck3y
Copy link
Collaborator Author

luck3y commented Jul 10, 2019

@jmtd Any objections to me merging this?

@jmtd jmtd merged commit 9cac0aa into jboss-openshift:master Jul 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants