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

Issue 1411: Update the EclipseLink 2.6 and 2.7 binaries #1461

Merged

Conversation

daliasheasha
Copy link
Contributor

@daliasheasha daliasheasha commented Jan 5, 2018

Updated the 2.6 binaries and scripts to accommodate the changes that were made to Liberty recently that did not allow us to take advantage of properties that were global before and now aren't anymore.

Updated the 2.7 binaries and scripts likewise. Also, updated the 2.7 binaries to use the EclipseLink 2.7 service branch instead of the 2.7_WAS branch.

After spending some time looking at the EclipseLink messages, I noticed that we were detecting a lot more messages than expected between EclipseLink 2.6 and 2.7. It turned out that a change went into EclipseLink a while back that combined logging messages and trace messages into one file which were previously separated. The separation allowed us to exclude trace messages from translation since there is no need to translate trace. So, I went back to the EclipseLink community and asked if we could revert that change since we still need the message separation to easily distinguish between messages that need translation and messages that don't. After getting their approval, I went ahead and reverted the change in EclipseLink. Currently, the EclipseLink 2.7 and master branch have their trace and logging messages separated. I also committed changes to the messages according to the ID review.
All these updates are currently in the EclipseLink repository. I am pulling that change into Liberty now with the suggested changes to the translation messages.

@daliasheasha daliasheasha self-assigned this Jan 5, 2018
@LibbyBot
Copy link

LibbyBot commented Jan 5, 2018

Code analysis and actions

DO NOT DELETE THIS COMMENT.
  • 5 product code files were changed.

  • Please describe in a separate comment how you tested your changes.

  • 3 messages files were changed and need an L2 review.

  • @OpenLiberty/message-reviewer Please review.

  • dev/com.ibm.websphere.appserver.thirdparty.eclipselink.2.7/resources/org/eclipse/persistence/internal/localization/i18n/ToStringLocalizationResource.nlsprops

  • dev/com.ibm.websphere.appserver.thirdparty.eclipselink.2.7/resources/org/eclipse/persistence/internal/localization/i18n/LoggingLocalizationResource.nlsprops

  • dev/com.ibm.websphere.appserver.thirdparty.eclipselink.2.7/resources/org/eclipse/persistence/exceptions/i18n/SDOExceptionResource.nlsprops

  • 3 NLS files were changed and need an ID review.

  • @OpenLiberty/message-reviewer Please review.

  • dev/com.ibm.websphere.appserver.thirdparty.eclipselink.2.7/resources/org/eclipse/persistence/internal/localization/i18n/ToStringLocalizationResource.nlsprops

  • dev/com.ibm.websphere.appserver.thirdparty.eclipselink.2.7/resources/org/eclipse/persistence/internal/localization/i18n/LoggingLocalizationResource.nlsprops

  • dev/com.ibm.websphere.appserver.thirdparty.eclipselink.2.7/resources/org/eclipse/persistence/exceptions/i18n/SDOExceptionResource.nlsprops

@daliasheasha
Copy link
Contributor Author

#build

@LibbyBot
Copy link

LibbyBot commented Jan 5, 2018

Your personal build request is at https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_8dC3sPG9Eee7FMdg7u8gzA

Target locations of links might be accessible only to IBM employees.

@LibbyBot
Copy link

LibbyBot commented Jan 5, 2018

The build daliasheasha-1461-20180105-0322
https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_8dC3sPG9Eee7FMdg7u8gzA
completed and has errors or failures.

@dazey3
Copy link
Contributor

dazey3 commented Jan 5, 2018

fixes #1411

Copy link
Contributor

@dazey3 dazey3 left a comment

Choose a reason for hiding this comment

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

The changes look good. Make sure you link your issue to this pull request for the nlsprop changes and test this for DHE users

@dazey3 dazey3 requested a review from jgrassel January 5, 2018 15:40
@daliasheasha
Copy link
Contributor Author

The build failed due to a known unrelated failure.

The updatebinaries.gradle scripts should run successfully as long as this change exists in the environment:
https://github.com/OpenLiberty/open-liberty/pull/1452/files#diff-687e8540545104f724086f8031c5d601

@bsbyrd1
Copy link
Member

bsbyrd1 commented Jan 5, 2018

Thanks, @daliasheasha. It looks like you have made trace/logging message updates based on the ID Review, and this issue (1461) does not require ID.

@daliasheasha
Copy link
Contributor Author

#build

@LibbyBot
Copy link

LibbyBot commented Jan 5, 2018

Your personal build request is at https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_Al5wwPI9Eee7FMdg7u8gzA

Target locations of links might be accessible only to IBM employees.

Copy link
Member

@cbridgha cbridgha left a comment

Choose a reason for hiding this comment

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

Looks good

@daliasheasha
Copy link
Contributor Author

@dazey3 @jgrassel I tested my original change for DHE and it failed since coordinates in oss_maven do not get mapped to DHE. So, I made a second commit that moves the eclipselink 2.7 coordinates back to oss_ibm.maven since only coordinates in that file are mapped to DHE.

@daliasheasha
Copy link
Contributor Author

fixes #332

@LibbyBot
Copy link

LibbyBot commented Jan 6, 2018

The build daliasheasha-1461-20180105-1835
https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_Al5wwPI9Eee7FMdg7u8gzA
completed successfully!

@OpenLiberty OpenLiberty deleted a comment from LibbyBot Jan 7, 2018
@daliasheasha daliasheasha merged commit b9f40db into OpenLiberty:integration Jan 7, 2018
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.

6 participants