[bridge/github] Correcte some types and add comments

General improvements

Co-Authored-By: Michael Muré <batolettre@gmail.com>

empty array check

an empty array is not nil

Co-Authored-By: Michael Muré <batolettre@gmail.com>

2 files changed, 32 insertions, 13 deletions

diff --git a/bridge/github/export.go b/bridge/github/export.go
index 1ab6ebca..9e394334 100644
--- a/bridge/github/export.go
+++ b/bridge/github/export.go
@@ -22,7 +22,7 @@ var (
 	ErrMissingIdentityToken = errors.New("missing identity token")
 )
 
-// githubImporter implement the Importer interface
+// githubExporter implement the Exporter interface
 type githubExporter struct {
 	conf core.Configuration
 
@@ -63,8 +63,11 @@ func (ge *githubExporter) Init(conf core.Configuration) error {
 	return nil
 }
 
+// allowOrigin verify that origin is allowed to get exported.
+// if the exporter was initialized with no specified origins, it will return
+// true for all origins
 func (ge *githubExporter) allowOrigin(origin string) bool {
-	if ge.onlyOrigins == nil {
+	if len(ge.onlyOrigins) == 0 {
 		return true
 	}
 
@@ -78,7 +81,7 @@ func (ge *githubExporter) allowOrigin(origin string) bool {
 }
 
 // getIdentityClient return an identity github api v4 client
-// if no client were found it will initilize it from the known tokens map and cache it for next it use
+// if no client were found it will initilize it from the known tokens map and cache it for next use
 func (ge *githubExporter) getIdentityClient(id string) (*githubv4.Client, error) {
 	client, ok := ge.identityClient[id]
 	if ok {
@@ -143,6 +146,7 @@ bugLoop:
 						return err
 					}
 
+					// avoid calling exportBug multiple times for the same bug
 					continue bugLoop
 				}
 			}
@@ -163,7 +167,7 @@ func (ge *githubExporter) exportBug(b *cache.BugCache, since time.Time) error {
 
 	// Special case:
 	// if a user try to export a bug that is not already exported to Github (or imported
-	// from Github) and he is not the author of the bug. There is nothing we can do.
+	// from Github) and we do not have the token of the bug author, there is nothing we can do.
 
 	// first operation is always createOp
 	createOp := snapshot.Operations[0].(*bug.CreateOperation)
@@ -184,6 +188,7 @@ func (ge *githubExporter) exportBug(b *cache.BugCache, since time.Time) error {
 			// if we find github ID, github URL must be found too
 			panic("expected to find github issue URL")
 		}
+
 		// will be used to mark operation related to a bug as exported
 		bugGithubID = githubID
 		bugGithubURL = githubURL
@@ -427,6 +432,7 @@ func (ge *githubExporter) getGithubLabelID(gc *githubv4.Client, label string) (s
 		return "", err
 	}
 
+	// if label id is empty, it means there is no such label in this Github repository
 	if q.Repository.Label.ID == "" {
 		return "", fmt.Errorf("label not found")
 	}
@@ -434,7 +440,7 @@ func (ge *githubExporter) getGithubLabelID(gc *githubv4.Client, label string) (s
 	return q.Repository.Label.ID, nil
 }
 
-func (ge *githubExporter) createGithubLabel(label, labelColor string) (string, error) {
+func (ge *githubExporter) createGithubLabel(label, color string) (string, error) {
 	url := fmt.Sprintf("%s/repos/%s/%s/labels", githubV3Url, ge.conf[keyOwner], ge.conf[keyProject])
 
 	client := &http.Client{
@@ -447,7 +453,7 @@ func (ge *githubExporter) createGithubLabel(label, labelColor string) (string, e
 		Description string `json:"description"`
 	}{
 		Name:  label,
-		Color: labelColor,
+		Color: color,
 	}
 
 	data, err := json.Marshal(params)
@@ -516,7 +522,7 @@ func (ge *githubExporter) getOrCreateGithubLabelID(gc *githubv4.Client, reposito
 		return labelID, nil
 	}
 
-	// hex color
+	// RGBA to hex color
 	rgba := label.RGBA()
 	hexColor := fmt.Sprintf("%.2x%.2x%.2x", rgba.R, rgba.G, rgba.B)
 
diff --git a/bridge/github/export_test.go b/bridge/github/export_test.go
index 85f5eb07..8f0f7869 100644
--- a/bridge/github/export_test.go
+++ b/bridge/github/export_test.go
@@ -161,19 +161,24 @@ func testCases(repo *cache.RepoCache, identity *cache.IdentityCache) ([]*testCas
 
 }
 
-func TestExporter(t *testing.T) {
+func TestPushPull(t *testing.T) {
+	// repo owner
 	user := os.Getenv("TEST_USER")
+
+	// token must have 'repo' and 'delete_repo' scopes
 	token := os.Getenv("GITHUB_TOKEN_ADMIN")
 	if token == "" {
 		t.Skip("Env var GITHUB_TOKEN_ADMIN missing")
 	}
 
+	// create repo backend
 	repo := repository.CreateTestRepo(false)
 	defer repository.CleanupTestRepos(t, repo)
 
 	backend, err := cache.NewRepoCache(repo)
 	require.NoError(t, err)
 
+	// set author identity
 	author, err := backend.NewIdentity("test identity", "test@test.org")
 	if err != nil {
 		t.Fatal(err)
@@ -195,13 +200,13 @@ func TestExporter(t *testing.T) {
 	// generate project name
 	projectName := generateRepoName()
 
-	// create repository
+	// create target Github repository
 	if err := createRepository(projectName, token); err != nil {
 		t.Fatal(err)
 	}
 	fmt.Println("created repository", projectName)
 
-	// delete repository before ending tests
+	// Make sure to remove the Github repository when the test end
 	defer func(t *testing.T) {
 		if err := deleteRepository(projectName, user, token); err != nil {
 			t.Fatal(err)
@@ -253,7 +258,9 @@ func TestExporter(t *testing.T) {
 			// so number of operations should double
 			require.Len(t, tt.bug.Snapshot().Operations, tt.numOrOp*2)
 
+			// verify operation have correcte metadata
 			for _, op := range tt.bug.Snapshot().Operations {
+				// Check if the originals operations (*not* SetMetadata) are tagged properly
 				if _, ok := op.(*bug.SetMetadataOperation); !ok {
 					_, haveIDMetadata := op.GetMetadata(keyGithubId)
 					require.True(t, haveIDMetadata)
@@ -263,21 +270,23 @@ func TestExporter(t *testing.T) {
 				}
 			}
 
+			// get bug github ID
 			bugGithubID, ok := tt.bug.Snapshot().Operations[0].GetMetadata(keyGithubId)
 			require.True(t, ok)
 
+			// retrive bug from backendTwo
 			importedBug, err := backendTwo.ResolveBugCreateMetadata(keyGithubId, bugGithubID)
 			require.NoError(t, err)
 
+			// verify bug have same number of original operations
 			require.Len(t, importedBug.Snapshot().Operations, tt.numOrOp)
 
+			// verify bugs are taged with origin=github
 			issueOrigin, ok := importedBug.Snapshot().Operations[0].GetMetadata(keyOrigin)
 			require.True(t, ok)
 			require.Equal(t, issueOrigin, target)
 
-			for _, _ = range importedBug.Snapshot().Operations {
-				// test operations or last bug state ?
-			}
+			//TODO: maybe more tests to ensure bug final state
 		})
 	}
 }
@@ -292,7 +301,9 @@ func generateRepoName() string {
 	return fmt.Sprintf("%s-%s", testRepoBaseName, string(b))
 }
 
+// create repository need a token with scope 'repo'
 func createRepository(project, token string) error {
+// This function use the V3 Github API because repository creation is not supported yet on the V4 API.
 	url := fmt.Sprintf("%s/user/repos", githubV3Url)
 
 	params := struct {
@@ -332,7 +343,9 @@ func createRepository(project, token string) error {
 	return resp.Body.Close()
 }
 
+// delete repository need a token with scope 'delete_repo'
 func deleteRepository(project, owner, token string) error {
+// This function use the V3 Github API because repository removal is not supported yet on the V4 API.
 	url := fmt.Sprintf("%s/repos/%s/%s", githubV3Url, owner, project)
 
 	req, err := http.NewRequest("DELETE", url, nil)