RFR: 1271: Webrev generation should be optional [v4]

Zhao Song zsong at openjdk.org
Tue Feb 21 19:31:40 UTC 2023


On Thu, 16 Feb 2023 21:35:55 GMT, Zhao Song <zsong at openjdk.org> wrote:

> This looks like it solves the stated problem, but the WebrevStorage is still initiated and will still do some work, like creating directories for webrev creation. Ideally I would like that whole part of the bot be optional, but it seems really hard to achieve without having to add null checks all over the place.
> 
> I'm not sure what the best solution is here. Maybe we can introduce another `WebrevDescription.Type.NONE` and if neither kind of webrev has been configured, then `WebrevGenerator::generate` would return a `Webrevdescription` of `NONE` type.
> 
> Another possible solution would be to have an `enabled()` method on `WebrevStorage` and wrap all uses of `webrevGenerator` in a check for that. That may get messy though, but would definitely reduce unnecessary work when webrevs are disabled.
> 
> Have you looked at `updateWebrevComment`. When webrevs are disabled, we shouldn't post that comment either.

Checked the logic again and I found although `WebrevStorage` is still initiated, it will just generate some paths but not create directories.

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

PR: https://git.openjdk.org/skara/pull/1475


More information about the skara-dev mailing list