Skip to content

feat(code-quality): add golangci-lint to CI and lint project #245

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 6 commits into from
Closed

feat(code-quality): add golangci-lint to CI and lint project #245

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
12da984
feat(code-quality): add golangci-lint to CI and lint project
darkweak Dec 20, 2022
7f28beb
feat(code-quality): move golangci-lint to PR only
darkweak Dec 21, 2022
4e63324
feat(code-quality): add error checks
darkweak Dec 21, 2022
1779fcd
fix from tucksaun review
darkweak Jan 15, 2023
90ab8cb
fix: review from tucksaun, wrap errors
darkweak Feb 14, 2023
8f5915c
fix ci errors
darkweak Feb 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .github/workflows/releaser.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ jobs:
echo '"go generate" changed some Go generated code, run "symfony cloud:self-update" then "go generate ./" locally and make a Pull Request with the changes'
git diff
exit 1
-
name: Run golangci-lint
uses: golangci/golangci-lint-action@v3
with:
only-new-issues: true
-
name: Test
run: go test -v ./...
Expand Down
16 changes: 16 additions & 0 deletions .golangci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
run:
timeout: 30s
issues-exit-code: 1

linters:
enable:
- gci
- wrapcheck

linters-settings:
wrapcheck:
ignorePackageGlobs:
# We already make sure your own packages wrap errors properly
- github.com/symfony-cli/*
errcheck:
ignore: github.com/symfony-cli/terminal:^(Ep|P)rint
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing newline at the end of the file

68 changes: 41 additions & 27 deletions book/checkout.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ import (
func (b *Book) Checkout(step string) error {
// FIXME: keep vendor/ node_modules/ around before git clean, but them back as they will be updated the right way, less Internet traffic
// FIXME: if the checkout is to a later step, no need to remove the DB, we can just migrate it
os.Chdir(b.Dir)
if err := os.Chdir(b.Dir); err != nil {
return errors.WithStack(err)
}
Comment on lines +38 to +40
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if err := os.Chdir(b.Dir); err != nil {
return errors.WithStack(err)
}
if err := errors.WithStack(os.Chdir(b.Dir)); err != nil {
return err
}

I'm not saying we have to do it, but we can do it when there's only a single return value, this also better preserve stack traces IIRC

step = strings.Replace(step, ".", "-", -1)
tag := fmt.Sprintf("step-%s", step)
branch := "work-" + tag
Expand Down Expand Up @@ -80,21 +82,21 @@ func (b *Book) Checkout(step string) error {

printBanner("<comment>[GIT]</> Removing Git ignored files (vendor, cache, ...)", b.Debug)
if err := executeCommand([]string{"git", "clean", "-d", "-f", "-x"}, b.Debug, false, nil); err != nil {
return err
return errors.WithStack(err)
}
printBanner("<comment>[GIT]</> Resetting Git staged files", b.Debug)
if err := executeCommand([]string{"git", "reset", "HEAD", "."}, b.Debug, false, nil); err != nil {
return err
return errors.WithStack(err)
}
printBanner("<comment>[GIT]</> Removing un-tracked Git files", b.Debug)
if err := executeCommand([]string{"git", "checkout", "."}, b.Debug, false, nil); err != nil {
return err
return errors.WithStack(err)
}

printBanner("<comment>[WEB]</> Adding .env.local", b.Debug)
emptyFile, err := os.Create(filepath.Join(b.Dir, ".env.local"))
if err != nil {
return err
return errors.WithStack(err)
}
emptyFile.Close()
if !b.Debug {
Expand All @@ -110,40 +112,44 @@ func (b *Book) Checkout(step string) error {
printBanner("<comment>[WEB]</> Stopping Docker Containers", b.Debug)
if hasDocker {
if err := executeCommand(append(dockerComposeBin(), "down", "--remove-orphans"), b.Debug, false, nil); err != nil {
return err
return errors.WithStack(err)
}
} else {
terminal.Println("Skipped for this step")
}

printBanner("<comment>[WEB]</> Stopping the Local Web Server", b.Debug)
executeCommand([]string{"symfony", "server:stop"}, b.Debug, true, nil)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would tend to think that @fabpot deliberately skip this one

if err = executeCommand([]string{"symfony", "server:stop"}, b.Debug, true, nil); err != nil {
return errors.WithStack(err)
}

printBanner("<comment>[WEB]</> Stopping the Platform.sh tunnel", b.Debug)
if err := executeCommand([]string{"symfony", "tunnel:close", "-y"}, b.Debug, true, nil); err != nil {
return err
return errors.WithStack(err)
}

printBanner("<comment>[GIT]</> Checking out the step", b.Debug)
if err := executeCommand([]string{"git", "checkout", "-B", branch, tag}, b.Debug, false, nil); err != nil {
return err
return errors.WithStack(err)
}

printBanner("<comment>[SPA]</> Stopping the Local Web Server", b.Debug)
if _, err := os.Stat(filepath.Join(b.Dir, "spa")); err == nil {
executeCommand([]string{"symfony", "server:stop", "--dir", filepath.Join(b.Dir, "spa")}, b.Debug, true, nil)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

if err = executeCommand([]string{"symfony", "server:stop", "--dir", filepath.Join(b.Dir, "spa")}, b.Debug, true, nil); err != nil {
return errors.WithStack(err)
}
} else {
terminal.Println("Skipped for this step")
}

printBanner("<comment>[WEB]</> Installing Composer dependencies (might take some time)", b.Debug)
if err := executeCommand([]string{"symfony", "composer", "install"}, b.Debug, false, nil); err != nil {
return err
return errors.WithStack(err)
}

printBanner("<comment>[WEB]</> Adding .env.local", b.Debug)
if emptyFile, err = os.Create(filepath.Join(b.Dir, ".env.local")); err != nil {
return err
return errors.WithStack(err)
}
emptyFile.Close()
if !b.Debug {
Expand All @@ -153,7 +159,7 @@ func (b *Book) Checkout(step string) error {
printBanner("<comment>[WEB]</> Starting Docker Compose", b.Debug)
if hasDocker {
if err := executeCommand(append(dockerComposeBin(), "up", "-d"), b.Debug, false, nil); err != nil {
return err
return errors.WithStack(err)
}
printBanner("<comment>[WEB]</> Waiting for the Containers to be ready", b.Debug)
if _, err := os.Stat(filepath.Join(b.Dir, "src", "MessageHandler", "CommentMessageHandler.php")); err == nil {
Expand All @@ -179,7 +185,7 @@ func (b *Book) Checkout(step string) error {
}
if hasMigrations {
if err := executeCommand([]string{"symfony", "console", "doctrine:migrations:migrate", "-n"}, b.Debug, false, nil); err != nil {
return err
return errors.WithStack(err)
}
} else {
terminal.Println("Skipped for this step")
Expand All @@ -188,7 +194,7 @@ func (b *Book) Checkout(step string) error {
printBanner("<comment>[WEB]</> Inserting Fixtures", b.Debug)
if _, err := os.Stat(filepath.Join(b.Dir, "src", "DataFixtures")); err == nil {
if err := executeCommand([]string{"symfony", "console", "doctrine:fixtures:load", "-n"}, b.Debug, false, nil); err != nil {
return err
return errors.WithStack(err)
}
} else {
terminal.Println("Skipped for this step")
Expand All @@ -202,7 +208,7 @@ func (b *Book) Checkout(step string) error {
args = []string{"yarn", "install"}
}
if err := executeCommand(args, b.Debug, false, nil); err != nil {
return err
return errors.WithStack(err)
}
} else {
terminal.Println("Skipped for this step")
Expand All @@ -215,38 +221,42 @@ func (b *Book) Checkout(step string) error {
args = []string{"yarn", "encore", "dev"}
}
if err := executeCommand(args, b.Debug, false, nil); err != nil {
return err
return errors.WithStack(err)
}
} else {
terminal.Println("Skipped for this step")
}

printBanner("<comment>[WEB]</> Starting the Local Web Server", b.Debug)
if err := executeCommand([]string{"symfony", "server:start", "-d"}, b.Debug, false, nil); err != nil {
return err
return errors.WithStack(err)
}

printBanner("<comment>[WEB]</> Starting Message Consumer", b.Debug)
if _, err := os.Stat(filepath.Join(b.Dir, "src", "MessageHandler", "CommentMessageHandler.php")); err == nil {
if err := executeCommand([]string{"symfony", "run", "-d", "--watch", "config,src,templates,vendor", "symfony", "console", "messenger:consume", "async", "-vv"}, b.Debug, false, nil); err != nil {
return err
return errors.WithStack(err)
}
} else {
terminal.Println("Skipped for this step")
}

printBanner("<comment>[SPA]</> Installing Node dependencies (might take some time)", b.Debug)
if _, err := os.Stat(filepath.Join(b.Dir, "spa")); err == nil {
os.Chdir(filepath.Join(b.Dir, "spa"))
if err = os.Chdir(filepath.Join(b.Dir, "spa")); err != nil {
return errors.WithStack(err)
}
args := []string{"npm", "install"}
if _, err := os.Stat(filepath.Join(b.Dir, "yarn.lock")); err == nil {
// old version of the book using Yarn instead of npm
args = []string{"yarn", "install"}
}
if err := executeCommand(args, b.Debug, false, nil); err != nil {
return err
return errors.WithStack(err)
}
if err = os.Chdir(b.Dir); err != nil {
return errors.WithStack(err)
}
os.Chdir(b.Dir)
} else {
terminal.Println("Skipped for this step")
}
Expand All @@ -264,24 +274,28 @@ func (b *Book) Checkout(step string) error {
if endpoint.String() == "" {
return errors.Errorf("unable to get the URL of the local web server:\n%s\n%s", stderr.String(), endpoint.String())
}
os.Chdir(filepath.Join(b.Dir, "spa"))
if err = os.Chdir(filepath.Join(b.Dir, "spa")); err != nil {
return errors.WithStack(err)
}
env := append(os.Environ(), "API_ENDPOINT="+endpoint.String())
args := []string{"npx", "encore", "dev"}
if _, err := os.Stat(filepath.Join(b.Dir, "yarn.lock")); err == nil {
args = []string{"yarn", "encore", "dev"}
}
if err := executeCommand(args, b.Debug, false, env); err != nil {
return err
return errors.WithStack(err)
}
if err = os.Chdir(b.Dir); err != nil {
return errors.WithStack(err)
}
os.Chdir(b.Dir)
} else {
terminal.Println("Skipped for this step")
}

printBanner("<comment>[SPA]</> Starting the Local Web Server", b.Debug)
if _, err := os.Stat(filepath.Join(b.Dir, "spa")); err == nil {
if err := executeCommand([]string{"symfony", "server:start", "-d", "--passthru", "index.html", "--dir", filepath.Join(b.Dir, "spa")}, b.Debug, false, nil); err != nil {
return err
return errors.WithStack(err)
}
} else {
terminal.Println("Skipped for this step")
Expand Down Expand Up @@ -322,7 +336,7 @@ func executeCommand(args []string, debug, skipErrors bool, env []string) error {
terminal.Println("<error>[ KO ]</>")
}
terminal.Print(buf.String())
return err
return errors.WithStack(err)
}
if !debug {
terminal.Println("<info>[ OK ]</>")
Expand Down
8 changes: 5 additions & 3 deletions book/clone.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func (b *Book) Clone(version string) error {
ui.Section("Checking Book Requirements")
ready, err := CheckRequirements()
if err != nil {
return err
return errors.WithStack(err)
}
terminal.Println("")
if !ready {
Expand All @@ -50,7 +50,9 @@ func (b *Book) Clone(version string) error {
}
terminal.Println("")

os.Chdir(b.Dir)
if err = os.Chdir(b.Dir); err != nil {
return errors.WithStack(err)
}
// checkout the first step by default
ui.Section("Getting Ready for the First Step of the Book")
if err := b.Checkout("3"); err != nil {
Expand All @@ -59,7 +61,7 @@ func (b *Book) Clone(version string) error {
terminal.Println("Re-run the command with <comment>--debug</> to get more information about the error")
terminal.Println("")
}
return err
return errors.WithStack(err)
}
return nil
}
4 changes: 2 additions & 2 deletions book/reqs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,12 @@ func CheckRequirements() (bool, error) {
// PHP
minv, err := version.NewVersion("8.1.0")
if err != nil {
return false, err
return false, errors.WithStack(err)
}
store := phpstore.New(util.GetHomeDir(), true, nil)
wd, err := os.Getwd()
if err != nil {
return false, err
return false, errors.WithStack(err)
}
v, _, _, _ := store.BestVersionForDir(wd)
if v == nil {
Expand Down
3 changes: 2 additions & 1 deletion commands/book_check_reqs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package commands

import (
"github.com/pkg/errors"
"github.com/symfony-cli/console"
"github.com/symfony-cli/symfony-cli/book"
"github.com/symfony-cli/terminal"
Expand All @@ -35,7 +36,7 @@ var bookCheckReqsCmd = &console.Command{

ready, err := book.CheckRequirements()
if err != nil {
return err
return errors.WithStack(err)
}
terminal.Println("")
if ready {
Expand Down
7 changes: 4 additions & 3 deletions commands/book_checkout.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package commands

import (
"github.com/pkg/errors"
"github.com/symfony-cli/console"
"github.com/symfony-cli/symfony-cli/book"
"github.com/symfony-cli/terminal"
Expand All @@ -40,7 +41,7 @@ var bookCheckoutCmd = &console.Command{
Action: func(c *console.Context) error {
dir, err := getProjectDir(c.String("dir"))
if err != nil {
return err
return errors.WithStack(err)
}

book := &book.Book{
Expand All @@ -50,7 +51,7 @@ var bookCheckoutCmd = &console.Command{
}
if !c.Bool("force") {
if err := book.CheckRepository(); err != nil {
return err
return errors.WithStack(err)
}
}
if err := book.Checkout(c.Args().Get("step")); err != nil {
Expand All @@ -59,7 +60,7 @@ var bookCheckoutCmd = &console.Command{
terminal.Println("Re-run the command with <comment>--debug</> to get more information about the error")
terminal.Println("")
}
return err
return errors.WithStack(err)
}
return nil
},
Expand Down
13 changes: 6 additions & 7 deletions commands/init_templating.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ package commands
import (
"fmt"
"io"
"io/ioutil"
"net/http"
"net/url"
"os"
Expand Down Expand Up @@ -194,19 +193,19 @@ func getTemplates(rootDirectory, chosenTemplateName string, minorPHPVersion stri
)

if isFile {
templateConfigBytes, err = ioutil.ReadFile(chosenTemplateName)
templateConfigBytes, err = os.ReadFile(chosenTemplateName)
} else {
var resp *http.Response
resp, err = http.Get(chosenTemplateName)
if err != nil {
return nil, err
return nil, errors.WithStack(err)
}
if resp.StatusCode >= 400 {
return nil, errors.Errorf("Got HTTP status code >= 400: %s", resp.Status)
}
defer resp.Body.Close()

templateConfigBytes, err = ioutil.ReadAll(resp.Body)
templateConfigBytes, err = io.ReadAll(resp.Body)
}

if err != nil {
Expand All @@ -219,7 +218,7 @@ func getTemplates(rootDirectory, chosenTemplateName string, minorPHPVersion stri

terminal.Logger.Info().Msg("Using template " + chosenTemplateName)
} else {
files, err := ioutil.ReadDir(directory)
files, err := os.ReadDir(directory)
if err != nil {
return nil, errors.Wrap(err, "could not read configuration templates")
}
Expand All @@ -240,7 +239,7 @@ func getTemplates(rootDirectory, chosenTemplateName string, minorPHPVersion stri
continue
}

templateConfigBytes, err := ioutil.ReadFile(filepath.Join(directory, file.Name()))
templateConfigBytes, err := os.ReadFile(filepath.Join(directory, file.Name()))
if err != nil {
if isTemplateChosen {
return nil, errors.Wrap(err, "could not apply configuration template")
Expand Down Expand Up @@ -275,7 +274,7 @@ func getTemplates(rootDirectory, chosenTemplateName string, minorPHPVersion stri
return nil, errors.New("no matching template found")
}

phpini, err := ioutil.ReadFile(filepath.Join(directory, "php.ini"))
phpini, err := os.ReadFile(filepath.Join(directory, "php.ini"))
if err != nil {
return nil, errors.New("unable to find the php.ini template")
}
Expand Down
Loading