RFR: 2400: Avoid overwriting mbox archive [v2]
Zhao Song
zsong at openjdk.org
Mon Nov 18 23:48:26 UTC 2024
On Mon, 18 Nov 2024 22:55:41 GMT, Erik Joelsson <erikj at openjdk.org> wrote:
>> Zhao Song has updated the pull request incrementally with one additional commit since the last revision:
>>
>> review comment
>
> forge/src/main/java/org/openjdk/skara/forge/HostedRepository.java line 111:
>
>> 109: * @param authorEmail Email of author and committer for commit
>> 110: */
>> 111: void writeFileContents(String filename, String content, Branch branch, String message, String authorName, String authorEmail, boolean createNewFile);
>
> Please update the javadoc with the new parameter and make it clear what should happen when set to true or false. If set to true, the call will fail if the file exists. If set to false, the call will fail if the file does not exist.
Will do
> forge/src/main/java/org/openjdk/skara/forge/github/GitHubRepository.java line 294:
>
>> 292:
>> 293: @Override
>> 294: public void writeFileContents(String filename, String content, Branch branch, String message, String authorName, String authorEmail, boolean createNewEmail) {
>
> This variable name doesn't make sense here.
Thanks for catching it! I was in a hurry when working on it
> test/src/main/java/org/openjdk/skara/test/TestHostedRepository.java line 232:
>
>> 230:
>> 231: @Override
>> 232: public void writeFileContents(String filename, String content, Branch branch, String message, String authorName, String authorEmail, boolean createNewFile) {
>
> This implementation should also enforce the new behavior so we catch any mistakes in testing.
Will fix it
-------------
PR Review Comment: https://git.openjdk.org/skara/pull/1696#discussion_r1847419833
PR Review Comment: https://git.openjdk.org/skara/pull/1696#discussion_r1847420953
PR Review Comment: https://git.openjdk.org/skara/pull/1696#discussion_r1847421339
More information about the skara-dev
mailing list