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