Re: RFR: 8278255: Add more warning text in ReentrantLock and ReentrantReadWriteLock [v2]
On Sat, 27 Apr 2024 11:52:18 GMT, Viktor Klang wrote: >> This is an attempt to be more clear about recommendations on Lock usage. > > Viktor Klang has updated the pull request incrementally with one additional > commit since the last revision: > > Update > src/java.base/share/classes/java/util/concurrent/locks/ReentrantReadWriteLock.java > > Co-authored-by: Pavel Rappo <32523691+pavelra...@users.noreply.github.com> Marked as reviewed by alanb (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18974#pullrequestreview-2051219304
Re: RFR: 8278255: Add more warning text in ReentrantLock and ReentrantReadWriteLock [v2]
On Sat, 27 Apr 2024 11:52:18 GMT, Viktor Klang wrote: >> This is an attempt to be more clear about recommendations on Lock usage. > > Viktor Klang has updated the pull request incrementally with one additional > commit since the last revision: > > Update > src/java.base/share/classes/java/util/concurrent/locks/ReentrantReadWriteLock.java > > Co-authored-by: Pavel Rappo <32523691+pavelra...@users.noreply.github.com> Marked as reviewed by prappo (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18974#pullrequestreview-2050367663
Re: RFR: 8278255: Add more warning text in ReentrantLock and ReentrantReadWriteLock [v2]
On Wed, 1 May 2024 10:03:25 GMT, Pavel Rappo wrote: >> The grammar rules prefer "that" because it is a restrictive clause, even >> though "which" might sound better. See for example: >> https://owl.purdue.edu/owl/general_writing/grammar/that_vs_which.html > >> The grammar rules prefer "that" because it is a restrictive clause, even >> though "which" might sound better. See for example: >> https://owl.purdue.edu/owl/general_writing/grammar/that_vs_which.html > > Thanks for that link, Doug. > > I understand that two "that"s that are that close to each other might sound a > bit weird. (Obviously, I exaggerated the issue in that preceding sentence.) > Can we maybe rephrase it then? > > > * // If code could throw, make sure that it is executed inside try block > @pavelrappo Ok to integrate? 樂 Oh, you shouldn't have waited for me. I think it's fine. Thanks! - PR Comment: https://git.openjdk.org/jdk/pull/18974#issuecomment-2104760009
Re: RFR: 8278255: Add more warning text in ReentrantLock and ReentrantReadWriteLock [v2]
On Wed, 1 May 2024 10:03:25 GMT, Pavel Rappo wrote: >> The grammar rules prefer "that" because it is a restrictive clause, even >> though "which" might sound better. See for example: >> https://owl.purdue.edu/owl/general_writing/grammar/that_vs_which.html > >> The grammar rules prefer "that" because it is a restrictive clause, even >> though "which" might sound better. See for example: >> https://owl.purdue.edu/owl/general_writing/grammar/that_vs_which.html > > Thanks for that link, Doug. > > I understand that two "that"s that are that close to each other might sound a > bit weird. (Obviously, I exaggerated the issue in that preceding sentence.) > Can we maybe rephrase it then? > > > * // If code could throw, make sure that it is executed inside try block @pavelrappo Ok to integrate? 樂 - PR Comment: https://git.openjdk.org/jdk/pull/18974#issuecomment-2104730094
Re: RFR: 8278255: Add more warning text in ReentrantLock and ReentrantReadWriteLock [v2]
On Sat, 27 Apr 2024 11:52:18 GMT, Viktor Klang wrote: >> This is an attempt to be more clear about recommendations on Lock usage. > > Viktor Klang has updated the pull request incrementally with one additional > commit since the last revision: > > Update > src/java.base/share/classes/java/util/concurrent/locks/ReentrantReadWriteLock.java > > Co-authored-by: Pavel Rappo <32523691+pavelra...@users.noreply.github.com> src/java.base/share/classes/java/util/concurrent/locks/ReentrantReadWriteLock.java line 162: > 160: * } > 161: * } > 162: * // Make sure that code that could throw is executed inside the > try block @pavelrappo Or something like: Suggestion: * // Potentially throwing code should be placed inside the try block - PR Review Comment: https://git.openjdk.org/jdk/pull/18974#discussion_r1586174747
Re: RFR: 8278255: Add more warning text in ReentrantLock and ReentrantReadWriteLock [v2]
On Tue, 30 Apr 2024 19:41:54 GMT, Doug Lea wrote: > The grammar rules prefer "that" because it is a restrictive clause, even > though "which" might sound better. See for example: > https://owl.purdue.edu/owl/general_writing/grammar/that_vs_which.html Thanks for that link, Doug. I understand that two "that"s that are that close to each other might sound a bit weird. (Obviously, I exaggerated the issue in that preceding sentence.) Can we maybe rephrase it then? * // If code could throw, make sure that it is executed inside try block - PR Comment: https://git.openjdk.org/jdk/pull/18974#issuecomment-2088240552
Re: RFR: 8278255: Add more warning text in ReentrantLock and ReentrantReadWriteLock [v2]
On Sat, 27 Apr 2024 11:52:18 GMT, Viktor Klang wrote: >> This is an attempt to be more clear about recommendations on Lock usage. > > Viktor Klang has updated the pull request incrementally with one additional > commit since the last revision: > > Update > src/java.base/share/classes/java/util/concurrent/locks/ReentrantReadWriteLock.java > > Co-authored-by: Pavel Rappo <32523691+pavelra...@users.noreply.github.com> The grammar rules prefer "that" because it is a restrictive clause, even though "which" might sound better. See for example: https://owl.purdue.edu/owl/general_writing/grammar/that_vs_which.html - PR Comment: https://git.openjdk.org/jdk/pull/18974#issuecomment-2086883300
Re: RFR: 8278255: add more warning text in ReentrantLock and ReentrantReadWriteLock [v2]
On Tue, 30 Apr 2024 12:38:02 GMT, Viktor Klang wrote: > @pavelrappo Wdyt? I like it, but maybe native speakers who also write clearly could double check that change from _which_ to _that_? In no particular order: @DougLea, @AlanBateman, @stuart-marks. - PR Comment: https://git.openjdk.org/jdk/pull/18974#issuecomment-2085246810
Re: RFR: 8278255: add more warning text in ReentrantLock and ReentrantReadWriteLock [v2]
On Sat, 27 Apr 2024 11:52:18 GMT, Viktor Klang wrote: >> This is an attempt to be more clear about recommendations on Lock usage. > > Viktor Klang has updated the pull request incrementally with one additional > commit since the last revision: > > Update > src/java.base/share/classes/java/util/concurrent/locks/ReentrantReadWriteLock.java > > Co-authored-by: Pavel Rappo <32523691+pavelra...@users.noreply.github.com> @pavelrappo Wdyt? - PR Comment: https://git.openjdk.org/jdk/pull/18974#issuecomment-2085221026
Re: RFR: 8278255: add more warning text in ReentrantLock and ReentrantReadWriteLock [v2]
> This is an attempt to be more clear about recommendations on Lock usage. Viktor Klang has updated the pull request incrementally with one additional commit since the last revision: Update src/java.base/share/classes/java/util/concurrent/locks/ReentrantReadWriteLock.java Co-authored-by: Pavel Rappo <32523691+pavelra...@users.noreply.github.com> - Changes: - all: https://git.openjdk.org/jdk/pull/18974/files - new: https://git.openjdk.org/jdk/pull/18974/files/74c937a1..e416c59a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18974=01 - incr: https://webrevs.openjdk.org/?repo=jdk=18974=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18974.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18974/head:pull/18974 PR: https://git.openjdk.org/jdk/pull/18974
