feat: repo health check (#16) #17

Merged
张泊明518370910136 merged 37 commits from file_check into master 2024-09-11 20:09:27 +08:00
4 changed files with 18 additions and 24 deletions
Showing only changes of commit dc6e564832 - Show all commits

View File

@ -77,8 +77,10 @@ func main() {
fmt.Printf("## Release Tag Check Failed:\n%s\n", err.Error())
}
// FIXME: for drone usage
err = healthcheck.VerifyDirectory(*rootDir, *adminDir)
if err != nil {
fmt.Printf("## Directory File Check Failed:\n%s\n", err.Error())
if adminDir != nil && *adminDir != "" {
zzjc123 marked this conversation as resolved Outdated

what should be fixed?

what should be fixed?

I think it is due to we can only test it when it is deployed on drone so I just skip it when running the code. If not I couldn't push the code.

I think it is due to we can only test it when it is deployed on drone so I just skip it when running the code. If not I couldn't push the code.

I think checking an empty value for skipping it is enough. No dir specified = no verify.

I think checking an empty value for skipping it is enough. No dir specified = no verify.
err = healthcheck.VerifyDirectory(*rootDir, *adminDir)
if err != nil {
fmt.Printf("## Directory File Check Failed:\n%s\n", err.Error())
}
}
}

View File

@ -6,6 +6,7 @@ import (
"os"
"path/filepath"
"regexp"
"strings"
)
// getForbiddens retrieves a list of forbidden files in the specified root directory.
@ -61,15 +62,12 @@ func ForbiddenCheck(rootDir string, regexList []string, repo string, droneBranch
var message string
if len(forbids) > 0 {
message += fmt.Sprint(103, "the following forbidden files were found: ")
for _, file := range forbids {
message += fmt.Sprint(file, ", ")
}
message += "\n\nTo fix it, first make a backup of your repository and then run the following commands:\nfor i in "
for _, file := range forbids {
message += fmt.Sprint(file, " ")
}
message += fmt.Sprint("; do git filter-repo --force --invert-paths --path \\\"\\$i\\\"; done\ngit remote add origin ", repo, "\ngit push --set-upstream origin ", droneBranch, " --force")
message = "The following forbidden files were found: " +
strings.Join(forbids, ", ") +
"\n\nTo fix it, first make a backup of your repository and then run the following commands:\nfor i in " +
strings.Join(forbids, " ") +
fmt.Sprint("; do git filter-repo --force --invert-paths --path \\\"\\$i\\\"; done\ngit remote add origin ",
repo, "\ngit push --set-upstream origin ", droneBranch, " --force")
return fmt.Errorf(message)
}
return nil

View File

@ -2,19 +2,12 @@ package healthcheck
import (
"fmt"
"strings"
"github.com/go-git/go-git/v5"
"github.com/go-git/go-git/v5/plumbing"
)
func catTags(all []string) (out string) {
out = ""
for _, str := range all {
out += str + " "
}
return out
}
func getTagsFromRepo(repoPath string) ([]string, error) {
repo, err := git.PlainOpen(repoPath)
if err != nil {
@ -63,8 +56,7 @@ func CheckReleases(repoPath string, category string, n int) error {
}
}
if !found {
tagList := catTags(tags)
return fmt.Errorf("Wrong release tag '%s'. Please use one of %s.", target, tagList)
return fmt.Errorf("Wrong release tag '%s'. Please use one of '%s'.", target, strings.Join(tags, "', '"))

Why is the target release tag wrong?

Why is the `target` release tag wrong?

I intended to mean release tag is missing or wrong.

I intended to mean release tag is missing or wrong.

Do we need to specify missing or wrong? I think only we need to inform students that their release is wrong.

Do we need to specify missing or wrong? I think only we need to inform students that their release is wrong.

Do we need to test if they submit extra tags?

Do we need to test if they submit extra tags?

I think extra is fine, as long as they don't miss any tags.

I think extra is fine, as long as they don't miss any tags.

Then just print the error on missing tags.

Then just print the error on missing tags.
}
return nil
}

View File

@ -57,14 +57,16 @@ func VerifyDirectory(rootDir, compareDir string) error {
relPath, _ := filepath.Rel(rootDir, path)
file2 := filepath.Join(compareDir, relPath)
if !fileExists(file2) {
return fmt.Errorf("File %s in %s is missing in %s. Please immediately revert your changes!\n", relPath, rootDir, compareDir)
return fmt.Errorf("File %s is missing. Please immediately revert your changes!\n",
filepath.Join(rootDir, relPath))
}
match, err := filesMatch(path, file2)
if err != nil {

remove it

remove it

I mean remove the os.Exit(1), and also the one on line 55.

I mean remove the `os.Exit(1)`, and also the one on line 55.
return fmt.Errorf("error matching files: %w", err)
}
if !match {
return fmt.Errorf("File %s in %s is not identical to %s. Please revert your changes or contact the teaching team if you have a valid reason for adjusting them.\n", relPath, rootDir, compareDir)
return fmt.Errorf("File %s is altered. Please revert your changes or contact the teaching team if you have a valid reason for adjusting them.\n",
filepath.Join(rootDir, relPath))

why not just return error on not all passed?

why not just return error on not all passed?
}
}
return nil