In message <o2h70c713191004200431s6b6fb572i30726915644cb...@mail.gmail.com>, Kevin Zhou writes: > > Thanks, Mark and Sebb > > On Tue, Apr 20, 2010 at 6:37 PM, sebb <seb...@gmail.com> wrote: > > > Also, consider whether the header field could be made final - it looks > > like it is only set in the constructor.
Kevin, I'm still not happy about your commit r935873. Your commit message says: Apply patch for HARMONY-6504: [classlib][luni]Harmony returns null for header related function when file URL is handled. This patch implements the header related functions [getHeaderField(int)/getHeaderFieldKey(int)/getHeaderField( String)/getLastModified()] to prevent Harmony to return null when file URL is handled. and contains the change: + private final LinkedHashMap<String, String> header; but Mohanraj's patch contains the change: + private LinkedHashMap<String, String> header; So, at the very least, your comment should have said that the commit was a modified version of the patch with an explanation for the modification either in the commit message or on the JIRA comment associated with the commit[0]. (You should probably give credit for the change in this case as well.) Personally, if a modification isn't required to integrate the patch then I prefer to see two commits. One with the patch and one with subsequent improvements. Regards, Mark. [0] See Oliver's commit r935528 of HARMONY-6476 for example.
