Re: RFR: 8138732: Rename @HotSpotIntrinsicCandidate to @IntrinsicCandidate and move it to the jdk.internal.vm.annotation package [v2]
On 23.09.20 07:13, David Holmes wrote: On Tue, 22 Sep 2020 08:43:04 GMT, Erik Gahlin wrote: Marked as reviewed by kvn (Reviewer). Have you run the JFR tests in test/jdk/jdk/jfr? @marschall Please do not force-push anything as it breaks the commit history in the PR and renders previous reviews/comments obsolete. There is no way for the reviewers to see what changed between the commit they reviewed and the commit now in the PR. To update to latest changes you should have just merged your branch with your (up-to-date) local master, committed that merge in your local branch and then pushed that commit to your Personal Fork. The skara tooling will flatten the series of commits in a PR into one single well-formed commit that is pushed to the master branch of the repo. My apologies, I was not aware of this, I won't make this mistake in the future again. I don't assume there is a way to fix this now. Philippe
Re: RFR: 8138732: Rename @HotSpotIntrinsicCandidate to @IntrinsicCandidate and move it to the jdk.internal.vm.annotation package [v2]
On 22.09.20 10:45, Erik Gahlin wrote: On Sat, 12 Sep 2020 00:19:00 GMT, Vladimir Kozlov wrote: Philippe Marschall has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 8138732: Rename HotSpotIntrinsicCandidate to IntrinsicCandidate Marked as reviewed by kvn (Reviewer). Have you run the JFR tests in test/jdk/jdk/jfr? I initially did not but now I have. jdk.jfr.event.runtime.TestModuleEvents was failing, I have updated it to what makes sense to me. Cheers Philippe
Re: RFR: 8138732: Rename @HotSpotIntrinsicCandidate to @IntrinsicCandidate and move it to the jdk.internal.vm.annotation package [v2]
On Tue, 22 Sep 2020 08:43:04 GMT, Erik Gahlin wrote: >> Marked as reviewed by kvn (Reviewer). > > Have you run the JFR tests in test/jdk/jdk/jfr? @marschall Please do not force-push anything as it breaks the commit history in the PR and renders previous reviews/comments obsolete. There is no way for the reviewers to see what changed between the commit they reviewed and the commit now in the PR. To update to latest changes you should have just merged your branch with your (up-to-date) local master, committed that merge in your local branch and then pushed that commit to your Personal Fork. The skara tooling will flatten the series of commits in a PR into one single well-formed commit that is pushed to the master branch of the repo. - PR: https://git.openjdk.java.net/jdk/pull/45
Re: RFR: 8138732: Rename @HotSpotIntrinsicCandidate to @IntrinsicCandidate and move it to the jdk.internal.vm.annotation package [v2]
On Sat, 12 Sep 2020 00:19:00 GMT, Vladimir Kozlov wrote: >> Philippe Marschall has refreshed the contents of this pull request, and >> previous commits have been removed. The >> incremental views will show differences compared to the previous content of >> the PR. The pull request contains one new >> commit since the last revision: >> 8138732: Rename HotSpotIntrinsicCandidate to IntrinsicCandidate > > Marked as reviewed by kvn (Reviewer). Have you run the JFR tests in test/jdk/jdk/jfr? - PR: https://git.openjdk.java.net/jdk/pull/45
Re: RFR: 8138732: Rename @HotSpotIntrinsicCandidate to @IntrinsicCandidate and move it to the jdk.internal.vm.annotation package [v2]
On Wed, 9 Sep 2020 09:49:44 GMT, Philippe Marschall wrote: >> Hello, newbie here >> >> I picked JDK-8138732 to work on because it has a "starter" label and I >> believe I understand what to do. >> >> - I tried to update the copyright year to 2020 in every file. >> - I decided to change `@since` from 9 to 16 since it is a new annotation >> name in a new package. >> - I tried to keep code changes to a minimum, eg not change to imports if >> fully qualified class names are used instead of >> imports. In some cases I did minor reordering of imports to keep them >> sorted alphabetically. >> - All tier1 tests pass. >> - One jpackage/jlink tier2 test fails but I don't believe it is related. >> - Some tier3 Swing tests fail but I don't think they are related. > > Philippe Marschall has refreshed the contents of this pull request, and > previous commits have been removed. The > incremental views will show differences compared to the previous content of > the PR. Marked as reviewed by kvn (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/45
Re: RFR: 8138732: Rename @HotSpotIntrinsicCandidate to @IntrinsicCandidate and move it to the jdk.internal.vm.annotation package [v2]
On Thu, 10 Sep 2020 17:59:54 GMT, Paul Sandoz wrote: >> Philippe Marschall has refreshed the contents of this pull request, and >> previous commits have been removed. The >> incremental views will show differences compared to the previous content of >> the PR. > > Marked as reviewed by psandoz (Reviewer). You should consider upstreaming Graal changes (jdk.internal.vm.compiler) into https://github.com/oracle/graal Otherwise next time we do "Update Graal" in JDK they will be overwritten. - PR: https://git.openjdk.java.net/jdk/pull/45
Re: RFR: 8138732: Rename @HotSpotIntrinsicCandidate to @IntrinsicCandidate and move it to the jdk.internal.vm.annotation package [v2]
On Fri, 11 Sep 2020 21:53:12 GMT, Vladimir Kozlov wrote: >> Marked as reviewed by psandoz (Reviewer). > > You should consider upstreaming Graal changes (jdk.internal.vm.compiler) into > https://github.com/oracle/graal > Otherwise next time we do "Update Graal" in JDK they will be overwritten. Changes look good. I ran hs-tier1 and hs-tier3 test jobs which run Graal tests and they passed. - PR: https://git.openjdk.java.net/jdk/pull/45
Re: RFR: 8138732: Rename @HotSpotIntrinsicCandidate to @IntrinsicCandidate and move it to the jdk.internal.vm.annotation package [v2]
On Wed, 9 Sep 2020 09:49:44 GMT, Philippe Marschall wrote: >> Hello, newbie here >> >> I picked JDK-8138732 to work on because it has a "starter" label and I >> believe I understand what to do. >> >> - I tried to update the copyright year to 2020 in every file. >> - I decided to change `@since` from 9 to 16 since it is a new annotation >> name in a new package. >> - I tried to keep code changes to a minimum, eg not change to imports if >> fully qualified class names are used instead of >> imports. In some cases I did minor reordering of imports to keep them >> sorted alphabetically. >> - All tier1 tests pass. >> - One jpackage/jlink tier2 test fails but I don't believe it is related. >> - Some tier3 Swing tests fail but I don't think they are related. > > Philippe Marschall has refreshed the contents of this pull request, and > previous commits have been removed. The > incremental views will show differences compared to the previous content of > the PR. The pull request contains one new > commit since the last revision: > 8138732: Rename HotSpotIntrinsicCandidate to IntrinsicCandidate Marked as reviewed by psandoz (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/45
Re: RFR: 8138732: Rename @HotSpotIntrinsicCandidate to @IntrinsicCandidate and move it to the jdk.internal.vm.annotation package [v2]
On Wed, 9 Sep 2020 09:49:44 GMT, Philippe Marschall wrote: >> Hello, newbie here >> >> I picked JDK-8138732 to work on because it has a "starter" label and I >> believe I understand what to do. >> >> - I tried to update the copyright year to 2020 in every file. >> - I decided to change `@since` from 9 to 16 since it is a new annotation >> name in a new package. >> - I tried to keep code changes to a minimum, eg not change to imports if >> fully qualified class names are used instead of >> imports. In some cases I did minor reordering of imports to keep them >> sorted alphabetically. >> - All tier1 tests pass. >> - One jpackage/jlink tier2 test fails but I don't believe it is related. >> - Some tier3 Swing tests fail but I don't think they are related. > > Philippe Marschall has refreshed the contents of this pull request, and > previous commits have been removed. The > incremental views will show differences compared to the previous content of > the PR. The pull request contains one new > commit since the last revision: > 8138732: Rename HotSpotIntrinsicCandidate to IntrinsicCandidate Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/45
Re: RFR: 8138732: Rename @HotSpotIntrinsicCandidate to @IntrinsicCandidate and move it to the jdk.internal.vm.annotation package [v2]
> Hello, newbie here > > I picked JDK-8138732 to work on because it has a "starter" label and I > believe I understand what to do. > > - I tried to update the copyright year to 2020 in every file. > - I decided to change `@since` from 9 to 16 since it is a new annotation name > in a new package. > - I tried to keep code changes to a minimum, eg not change to imports if > fully qualified class names are used instead of > imports. In some cases I did minor reordering of imports to keep them > sorted alphabetically. > - All tier1 tests pass. > - One jpackage/jlink tier2 test fails but I don't believe it is related. > - Some tier3 Swing tests fail but I don't think they are related. Philippe Marschall has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 8138732: Rename HotSpotIntrinsicCandidate to IntrinsicCandidate - Changes: - all: https://git.openjdk.java.net/jdk/pull/45/files - new: https://git.openjdk.java.net/jdk/pull/45/files/47328f4b..1c9dd9da Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=45&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=45&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/45.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/45/head:pull/45 PR: https://git.openjdk.java.net/jdk/pull/45
