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

Remove Java_com_ibm_jit_crypto_* from jithelpers uma object #255

Merged
merged 1 commit into from
Oct 5, 2017

Conversation

vsebe
Copy link
Contributor

@vsebe vsebe commented Oct 5, 2017

Move crypto exports to vendors jcl exports files.
Update module.xml to include the export file when available.
Fixed uma exports parser.
This fix is related to issue: #55.

Signed-off-by: Violeta Sebe [email protected]

@@ -64,7 +64,12 @@ public void addArtifact(Artifact artifact) throws UMAException {
}

public void addExports(String group, Exports exps) {
exports.put(group, exps);
if ( exports.keySet().contains(group) ) {
Copy link
Member

Choose a reason for hiding this comment

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

containsKey() would also work

@@ -64,7 +64,12 @@ public void addArtifact(Artifact artifact) throws UMAException {
}

public void addExports(String group, Exports exps) {
exports.put(group, exps);
if ( exports.keySet().contains(group) ) {
exps.addExports(exports.get(group));
Copy link
Member

Choose a reason for hiding this comment

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

To avoid unexpected problems, and just as good practice, please create a new Exports rather than modifying the parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pshipton Fixed as suggested, thx!

if ( exports.containsKey(group) ) {
Exports allGroupExports = exports.get(group);
allGroupExports.addExports(exps);
exports.replace(group, allGroupExports);
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be needed, since the Exports in the Map was already modified on the previous line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, corrected, thx!

Move crypto exports to vendors jcl exports files
Update module.xml to include the export file when available
Fixed uma exports parser

Signed-off-by: Violeta Sebe <[email protected]>
@pshipton
Copy link
Member

pshipton commented Oct 5, 2017

jenkins test sanity plinux

@pshipton pshipton merged commit 2f26cc3 into eclipse-openj9:master Oct 5, 2017
@vsebe vsebe deleted the openj9.jcl branch October 12, 2017 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants