Implemented CI Handler for adding CI files to a repo #48
Conversation
| BASE_FILES_PATH string = "../../precommit/" | ||
| // pre-commit config file | ||
| PRE_COMMIT_CONFIG string = "../../precommit/CI/pre-commit-config.yaml" |
There was a problem hiding this comment.
Let's keep the style consistent, maybe change it to camel case as is with the other variables?
| // Logdir is the directory where all logs will be stored | ||
| logDir string |
There was a problem hiding this comment.
Why do we need a separate logDir, can't we use the existing logger?
| Python string `json:"python"` | ||
| Golang string `json:"golang"` | ||
| Node string `json:"node"` | ||
| Ts string `json:"ts"` | ||
| Flutter string `json:"flutter"` | ||
| Dart string `json:"dart"` | ||
| Docker string `json:"docker"` | ||
| Shell string `json:"shell"` |
There was a problem hiding this comment.
Why do we need all their types to be string. Can't we unmarshall to bool instead from the json request?
| ciList = append(ciList, "Default") | ||
| v := reflect.ValueOf(ci) | ||
| for i := 0; i < v.NumField(); i++ { | ||
| if v.Field(i).String() == "true" { |
There was a problem hiding this comment.
If CIAction can have bool types this should be changed accordingly.
| ciList = append(ciList, v.Type().Field(i).Name) | ||
| } | ||
| } | ||
| addChecks(ciList) |
There was a problem hiding this comment.
Can this function call never return an error?
| f, err := os.OpenFile(PRE_COMMIT_CONFIG, os.O_APPEND|os.O_WRONLY, 0600) | ||
| if err != nil { | ||
| log.Error("Error opening file:pre-commit.config") | ||
| return |
There was a problem hiding this comment.
Maybe return the err from here
| _, err = fmt.Fprintf(f, string(content)+"\n") | ||
| if err != nil { | ||
| fmt.Printf("Error writing to file:%s\n", PRE_COMMIT_CONFIG) | ||
| return |
| // combine the data of all files in ciList to a file named .pre-commit-config.yaml | ||
|
|
||
| // create the file even if it exists | ||
| os.Create(PRE_COMMIT_CONFIG) |
There was a problem hiding this comment.
I am not very comfortable with the idea of using the same file PRE_COMMIT_CONFIG for every call to addChecks. Imagine if there were two consecutive requests for adding CI checks (maybe the user was impatient, and didn't like to wait). Then this file can be overwritten while ci.sh is being executed and this can lead to hard-to-spot bugs. I would suggest some alternatives, you are free to implement any one of these.
- Use locks. Basically you need to ensure that the sequence of operations addChecks -> go addCI() is done atomically. So you would have to acquire a lock in the HTTP handler itself, which is not a very efficient solution imo.
- Have different files. Just don't forget to clean them up.
- You can change the file name PRE_COMMIT_CONFIG to have a repo signature. Something like ->
PRE_COMMIT_CONFIG + "_" + repoName. The script ci.sh would have to copy only this file instead of the entire directory. - You can create a temp directory (which will have an arbitrary name) where you can write PRE_COMMIT_CONFIG, and pass the temp directory name to
ci.shso that it would copy the entire directory as it is.
- You can change the file name PRE_COMMIT_CONFIG to have a repo signature. Something like ->
Added a CIHandler endpoint for adding pre-commit CI checks depending on the call from GitHub Action.
Created pre-commit files for Python, Node and Golang.
I have made a script for adding the final pre-commit file into the specified repository.