RFR: 1910: Basic support for Bitbucket as a Forge [v2]
Erik Joelsson
erikj at openjdk.org
Tue May 16 20:14:28 UTC 2023
On Tue, 16 May 2023 19:00:01 GMT, Erik Helin <ehelin at openjdk.org> wrote:
>> Erik Joelsson has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Review comments
>
> forge/src/main/java/org/openjdk/skara/forge/bitbucket/BitbucketHost.java line 55:
>
>> 53: this.sshPort = sshPort;
>> 54: this.credential = null;
>> 55: }
>
> It seems like credential cannot be `null` when `useSsh` is `true` (see the method `sshHostString`). Maybe the constructors should reflect this? Or use static factory methods like `BitBucketHost.withSSH` and/or `BitBucketHost.withPAT`?
I more or less copy pasted the existing gitlab implementation here so this is following the same existing patterns. The same relationship between useSsh and credential are present there too. I agree it could be made neater with a builder pattern like most other factories do, or static factory methods.
> forge/src/main/java/org/openjdk/skara/forge/bitbucket/BitbucketRepository.java line 51:
>
>> 49:
>> 50: public class BitbucketRepository implements HostedRepository {
>> 51: private final BitbucketHost bitbucketHost;
>
> I would have named the field just `host`.
Right, again copy paste and just mindlessly modifying the field names. This I will adjust.
-------------
PR Review Comment: https://git.openjdk.org/skara/pull/1521#discussion_r1195646665
PR Review Comment: https://git.openjdk.org/skara/pull/1521#discussion_r1195647115
More information about the skara-dev
mailing list