Skip to content

Conversation

@adinn
Copy link
Collaborator

@adinn adinn commented Jun 5, 2017

I have retested this patch on AArch64 against a clean install of netbeans for jdk9 (including removingthe old project dirs) and it now seems to work ok. So, I believe the problems I was seeing before were to do with out of date data in the binary files and this patch is good to include.

@tkrodriguez tkrodriguez self-assigned this Jun 6, 2017
@dougxc
Copy link
Member

dougxc commented Jun 7, 2017

@adinn , now that 93772f9 has been merged, can you please rebase this PR on it to see if the Travis gate now passes. Sorry for the inconvenience.

@adinn adinn closed this Jun 7, 2017
@adinn adinn deleted the aarch64_address_lowering branch June 7, 2017 09:52
@adinn
Copy link
Collaborator Author

adinn commented Jun 7, 2017

Hi Doug. New PR has been posted at #223

@dougxc
Copy link
Member

dougxc commented Jun 7, 2017

There is no need to close this PR and in general it's not a good idea as review continuity and context can be lost. Could you please (force) push your rebased changes to this PR.

@adinn
Copy link
Collaborator Author

adinn commented Jun 7, 2017 via email

@tkrodriguez
Copy link
Member

The normal practice is to use git pull --rebase to bring in the new changes and then use git push -f to push the rebased branch to github. It's really no different than pushing additional changes apart from the -f option which says to replace the current HEAD with the new HEAD. The PR just tracks the contents of the branch so whatever is in the branch is the contents of the PR.

@adinn
Copy link
Collaborator Author

adinn commented Jun 7, 2017

Hmm, ok. Thanks for the tip.

So, given that I have nuked the branch that this PR was cloned from and then recreated it from is this PR still valid? Or is the new PR cloned from the new branch required?

@tkrodriguez
Copy link
Member

The reopen button on this one is grayed out and a tooltip says it because there's already an open PR. I tried closing the new one and then the tooltip changes to something about the branch being recreated but is still grayed out. So I think we have to move to the new one. We're not losing much in this case but in the future try and keep it within a single PR. We can always help out with any git trickery required to make it work.

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.

3 participants