Community
Participate
Working Groups
Build ID: xx Steps To Reproduce: try { } finally { } @@@@ lock.lock(); try { } finally { lock.unlock(); } @@@@ Both patterns are very common in concurrent programming. More information:
Created attachment 65025 [details] fix also removes two unneccessary spaces in properties file
Created attachment 65028 [details] forgot the "lock"'s id
*** Bug 37825 has been marked as a duplicate of this bug. ***
*** Bug 373471 has been marked as a duplicate of this bug. ***
New Gerrit change created: https://git.eclipse.org/r/44787
In context of hte #greatfix initiative I have pushed a branch for review with these two templates, based on the proposed templates, but for "java-statements" context instead of "java" context, as these do not make any sense outside the java-statements scope. Please assign me and add the greatfix keyword.
(In reply to Robert Roth from comment #6) > In context of hte #greatfix initiative I have pushed a branch for review > with these two templates, based on the proposed templates, but for > "java-statements" context instead of "java" context, as these do not make > any sense outside the java-statements scope. > > Please assign me and add the greatfix keyword. https://git.eclipse.org/r/44787 cannot be considered for greatfix as most of the implementation was already available as a patch in comment #2. This can be committed as your contribution once the following issues are handled: - Fix test failures and update the tests by adding new tests for the new templates. - See duplicate bug 373471 which needs the new template 'finally block' to just add the keyword 'finally' with a block - similar to template for "else block". - The other template (added in this patch) could be 'try finally' that adds try finally block - similar to template for "try catch block". - Update copyright header in the files. See https://wiki.eclipse.org/JDT_UI/How_to_Contribute#Coding_Conventions. - The template for lock looks fine.
I didn't know that fixing patches and getting them accepted doesn't count as a greatfix, but it totally makes sense. Thanks for the comments on the proposal though, I knew about the failing tests since I saw the logs, but I'm having a hard time correctly setting up a testing environment, but the other comments are new and useful for me, next time I won't forget updating the headers. I'll update the gerrit request once I manage to fix the failing tests and the rest of the issues.
(In reply to Robert Roth from comment #8) > I didn't know that fixing patches and getting them accepted doesn't count as > a greatfix, but it totally makes sense. Thanks for the comments on the > proposal though, I knew about the failing tests since I saw the logs, but > I'm having a hard time correctly setting up a testing environment, but the > other comments are new and useful for me, next time I won't forget updating > the headers. > I'll update the gerrit request once I manage to fix the failing tests and > the rest of the issues. Sure. See https://wiki.eclipse.org/JDT_UI/How_to_Contribute for setup and other details.
Fixed all issues mentioned above (hopefully I didn't miss anything), and rebased to master. Please let me know if you see anything else to fix in the updated patchset.
Gerrit change https://git.eclipse.org/r/44787 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=b7707ee96000dc96768ec554bc7ef41a2c90c7e8
(In reply to Robert Roth from comment #10) > Fixed all issues mentioned above (hopefully I didn't miss anything), and > rebased to master. Please let me know if you see anything else to fix in the > updated patchset. Thanks, I made the following changes and released the patch: - Copyright year should be incremented instead of adding the year again in all the modified files. - Renamed the existing 'try' template to 'try_catch'. - Renamed the new 'tryfinally' template to 'try_finally'. - TODO is not required in 'finally' template. Similar to the 'else' template. - In AdvancedQuickAssistTest, moved the tests for new templates at the end of each test method. (In reply to Eclipse Genie from comment #11) > Gerrit change https://git.eclipse.org/r/44787 was merged to [master]. > Commit: > http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/ > ?id=b7707ee96000dc96768ec554bc7ef41a2c90c7e8
