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.

Sign up for GitHub

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

Jump to bottom

Relocate createOrFindClonedNode, _uncommmonedNodes to OpenJ9 #2269

Closed
wants to merge 1 commit into from
Closed

Relocate createOrFindClonedNode, _uncommmonedNodes to OpenJ9 #2269

wants to merge 1 commit into from

Conversation

shivammittal99
Copy link
Contributor

Closes: #2068

Signed-off-by: Shivam Mittal [email protected]

@shivammittal99 shivammittal99 mentioned this pull request Jan 28, 2018
Closed
@0xdaryl 0xdaryl self-assigned this Jan 31, 2018
0xdaryl
0xdaryl requested changes Feb 13, 2018
View reviewed changes
Copy link
Contributor

@0xdaryl 0xdaryl left a comment

Choose a reason for hiding this comment

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

Your code changes look good.

Would you mind adding "to OpenJ9" to the end of your commit title? Since a title is all you provided for the commit I think it needs a little more context. Thanks.

@shivammittal99 shivammittal99 force-pushed the relocate_createOrFindClonedNode branch from 24af91f to abe8503 Compare February 15, 2018 11:44
@shivammittal99 shivammittal99 force-pushed the relocate_createOrFindClonedNode branch from abe8503 to 0b51892 Compare April 28, 2018 16:19
@shivammittal99 shivammittal99 force-pushed the relocate_createOrFindClonedNode branch from 0b51892 to 7239fcc Compare June 28, 2018 13:52
shivammittal99 added a commit to shivammittal99/openj9 that referenced this pull request Jun 28, 2018
@shivammittal99
Relocate createOrFindClonedNode, _uncommonedNodes from OMR
0cbf2cc 
OMR Issue: eclipse-omr/omr#2068
OMR PR: eclipse-omr/omr#2269

Signed-off-by: Shivam Mittal <[email protected]>
@charliegracie
Copy link
Contributor

ping @0xdaryl and @mstoodle

@charliegracie
Copy link
Contributor

@genie-omr build all

@mstoodle
Copy link
Contributor

mstoodle commented Aug 24, 2018 •
edited
Loading

there's a dependent PR at OpenJ9 which seems like it would be needed or else merging this could break openj9, so I'll be nice to them and not "just" merge this one even though it seems ready.

In future, maybe we could try to stage changes like this by first making the OMR code #ifdef J9_PROJECT_SPECIFIC, then copy it to OpenJ9, and then remove it from OMR. I know that's more pull requests and more work on the writing of commits, but it would certainly simplify and streamline the process to merge this kind of change. Not saying that excuses 7 months of delay to get it merged though :( .

0xdaryl
0xdaryl requested changes Apr 25, 2019
View reviewed changes
Copy link
Contributor

@0xdaryl 0xdaryl left a comment

Choose a reason for hiding this comment

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

This PR needs to be rebased and marked WIP until the dependent downstream changes (eclipse-openj9/openj9#1040) are merged.

@0xdaryl
Copy link
Contributor

0xdaryl commented May 22, 2019

Thank you for the contribution. This work was subsumed by #3857. Closing.

@0xdaryl 0xdaryl closed this May 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xdaryl 0xdaryl 0xdaryl requested changes

Assignees

@0xdaryl 0xdaryl

Labels
comp:compiler
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Relocate createOrFindClonedNode to OpenJ9
4 participants
@shivammittal99 @charliegracie @mstoodle @0xdaryl