Improve GitLab API finder error handling #569
Conversation
| @@ -37,7 +37,7 @@ class Gitlab { | |||
| } | |||
|
|
|||
| async repoBase() { | |||
| if (this._detected_base) return this._detected_base; | |||
| if (this.detectedBase) return this.detectedBase; | |||
There was a problem hiding this comment.
could still keep the underscore prefix though if the intention was to be a private var.
There was a problem hiding this comment.
More conventional than anything else
There was a problem hiding this comment.
The most idiomatic alternative would be using a private field, but that's just a proposal.
There was a problem hiding this comment.
We use that variable to memoi*e the function result, so it can be considered private.
There was a problem hiding this comment.
it does not have to be private either but should be exposed in other drivers for consisitency, being very purist we should do a closure of request here with the detectedbase... not needed IMHO
There was a problem hiding this comment.
Using a closure sounds tempting, but would add a considerable maintenance overhead and nobody cares about our internal API, yet. 😅 Let's leave it be for now.
There was a problem hiding this comment.
@casperdcl, what do you think?
There was a problem hiding this comment.
yes not really massive priority to do this at enterprise level yet, fine as-is
| @@ -37,7 +37,7 @@ class Gitlab { | |||
| } | |||
|
|
|||
| async repoBase() { | |||
| if (this._detected_base) return this._detected_base; | |||
| if (this.detectedBase) return this.detectedBase; | |||
There was a problem hiding this comment.
yes not really massive priority to do this at enterprise level yet, fine as-is
Closes #472, related to #563