[cr] RFR: 988: Skara webrev is empty if patch has deleted files [v2]

Erik Joelsson erikj at openjdk.java.net
Fri Apr 23 20:17:30 UTC 2021


On Fri, 23 Apr 2021 20:14:47 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

>> If a file in the patch has been deleted this causes a dereference of an undefined value. To fix this I removed the quick return in fetchFileContent so that this function always returns a value.
>> 
>> While verifying this, I also noted that the deleted file gets a Raw link which results in a 404. I think it should just not print the Raw link so I removed that too.
>> 
>> If a file has only been renamed, with no other changes, the patch will be an empty string. This upsets the hunk loop, so I added a case for handling this.
>> 
>> The resulting webrevs from the two reported patches look correct to me with these changes, but I would appreciate some more eyes on this.
>
> Erik Joelsson has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Adjusted for review comments

> @erikj79 I don't follow how removing the fast return is correct? The `fetch` will then result in an `404` which I don't think the rest of the code will handle? I _think_ returning `Promise.resolve([])` (i.e. an empty array wrapped in promise) would be better. `apply` should be able to handle that `lines` is empty, so things "should" just work.

Now that I know what a Promise is, then yes, this makes a lot more sense, thanks!

-------------

PR: https://git.openjdk.java.net/cr/pull/14


More information about the skara-dev mailing list