klesh commented on code in PR #4359:
URL: 
https://github.com/apache/incubator-devlake/pull/4359#discussion_r1101443056


##########
backend/helpers/pluginhelper/api/api_client.go:
##########
@@ -178,6 +187,26 @@ func (apiClient *ApiClient) GetTimeout() time.Duration {
        return apiClient.client.Timeout
 }
 
+// SetData FIXME ...
+func (apiClient *ApiClient) SetData(name string, data interface{}) {
+       apiClient.data_mutex.Lock()
+

Review Comment:
   Why empty lines?



##########
backend/helpers/pluginhelper/api/api_client.go:
##########
@@ -178,6 +187,26 @@ func (apiClient *ApiClient) GetTimeout() time.Duration {
        return apiClient.client.Timeout
 }
 
+// SetData FIXME ...

Review Comment:
   Please fill the doc



##########
backend/plugins/gitlab/models/connection.go:
##########
@@ -18,19 +18,96 @@ limitations under the License.
 package models
 
 import (
-       helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api"
+       "fmt"
+       "net/http"
+
+       "github.com/apache/incubator-devlake/core/errors"
+       "github.com/apache/incubator-devlake/helpers/pluginhelper/api"
+       
"github.com/apache/incubator-devlake/helpers/pluginhelper/api/apihelperabstract"
 )
 
 // GitlabConn holds the essential information to connect to the Gitlab API
 type GitlabConn struct {
-       helper.RestConnection `mapstructure:",squash"`
-       helper.AccessToken    `mapstructure:",squash"`
+       api.RestConnection `mapstructure:",squash"`
+       api.AccessToken    `mapstructure:",squash"`
+}
+
+const GitlabApiClientData_UserId string = "UserId"
+const GitlabApiClientData_ApiVersion string = "ApiVersion"
+
+// this function is used to rewrite the same function of AccessToken
+func (conn *GitlabConn) SetupAuthentication(request *http.Request) 
errors.Error {

Review Comment:
   No needed, please remove this function



##########
backend/plugins/gitlab/tasks/mr_extractor.go:
##########
@@ -155,7 +171,24 @@ func ExtractApiMergeRequests(taskCtx 
plugin.SubTaskContext) errors.Error {
                return errors.Convert(err)
        }
 
-       return extractor.Execute()
+       err = extractor.Execute()
+       if err != nil {
+               return err
+       }
+
+       if needDetail {

Review Comment:
   No, don't collect data during extraction.
   Please define another set of subtasks and check if they need to be actually 
executed inside subtasks



##########
backend/plugins/gitlab/tasks/pipeline_extractor.go:
##########
@@ -112,5 +117,23 @@ func ExtractApiPipelines(taskCtx plugin.SubTaskContext) 
errors.Error {
                return err
        }
 
-       return extractor.Execute()
+       err = extractor.Execute()
+       if err != nil {
+               return err
+       }
+
+       // If lost the detail info it means we have in the old version gitlab
+       // we should collect and Extract the detail for it
+       if needDetail {
+               err = CollectApiPipelineDetails(taskCtx)

Review Comment:
   Same as above



##########
backend/plugins/gitlab/models/connection.go:
##########
@@ -49,3 +131,5 @@ type ApiUserResponse struct {
 func (GitlabConnection) TableName() string {
        return "_tool_gitlab_connections"
 }
+
+type AeAppKey api.AppKey

Review Comment:
   Why? I believe this is to be deleted



##########
backend/helpers/pluginhelper/api/api_client.go:
##########
@@ -349,6 +378,20 @@ func UnmarshalResponse(res *http.Response, v interface{}) 
errors.Error {
        return nil
 }
 
+// UnmarshalResponseXML FIXME ...

Review Comment:
   Doc



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to