Skip to content

Conversation

@zavla
Copy link
Contributor

@zavla zavla commented Mar 17, 2020

No description provided.

}

files, err := ioutil.ReadDir(directory)
dir := filepath.Join(driver.BaseDir, directory)

Choose a reason for hiding this comment

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

assignments should only be cuddled with other assignments (from wsl)

}
return strings.ReplaceAll(s, "\"", `""`)

}

Choose a reason for hiding this comment

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

block should not end with a whitespace (or comment) (from wsl)

if strings.Index(s, "\"") == -1 {
return s
}
return strings.ReplaceAll(s, "\"", `""`)

Choose a reason for hiding this comment

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

return statements should not be cuddled if block has more than two lines (from wsl)

type args struct {
s string
}
tests := []struct {

Choose a reason for hiding this comment

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

assignments should only be cuddled with other assignments (from wsl)

{"1", args{` one" quote`}, ` one"" quote`},
{"1", args{` two"" quote`}, ` two"""" quote`},
}
for _, tt := range tests {

Choose a reason for hiding this comment

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

only one cuddle assignment allowed before range statement (from wsl)

return s
}
return strings.ReplaceAll(s, "\"", `""`)

Choose a reason for hiding this comment

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

unnecessary trailing newline (from whitespace)

}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := qoutedoubling(tt.args.s); got != tt.want {

Choose a reason for hiding this comment

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

Using the variable on range scope tt in function literal (from scopelint)

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := qoutedoubling(tt.args.s); got != tt.want {
t.Errorf("qoutedoubling() = %v, want %v", got, tt.want)

Choose a reason for hiding this comment

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

Using the variable on range scope tt in function literal (from scopelint)

"gopkg.in/dutchcoders/goftp.v1"

"github.com/fclairamb/ftpserver/server"
"gopkg.in/dutchcoders/goftp.v1"

Choose a reason for hiding this comment

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

File is not goimports-ed with -local github.com/fclairamb/ftpserver (from goimports)

Suggested change
"gopkg.in/dutchcoders/goftp.v1"
"gopkg.in/dutchcoders/goftp.v1"
"github.com/fclairamb/ftpserver/server"

}

func qoutedoubling(s string) string {
if strings.Index(s, "\"") == -1 {

Choose a reason for hiding this comment

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

S1003: should use !strings.Contains(s, """) instead (from gosimple)

prepare.bat Outdated
@@ -0,0 +1,7 @@
go get -t -v
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove this file

)
}

func qoutedoubling(s string) string {
Copy link
Owner

Choose a reason for hiding this comment

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

Please rename it quoteDoubling

Copy link
Owner

Choose a reason for hiding this comment

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

@zavla you still haven't fixed that

if fileName == " with spaces " {
expectedfileName := " with spaces "
if runtime.GOOS == "windows" {
expectedfileName = " with spaces"
Copy link
Owner

Choose a reason for hiding this comment

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

Is it not possible to have a space at the end of a file in windows ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello. No, such files are inaccessible.
https://docs.microsoft.com/en-gb/windows/win32/fileio/naming-a-file?redirectedfrom=MSDN#naming_conventions

Do not end a file or directory name with a space or a period. Although the underlying file system may support such names, the Windows shell and user interface does not.

@fclairamb
Copy link
Owner

These changes seem reasonable for the most part. What is blocking the build is that you are using strings.ReplaceAll which is a go 1.12+ API. But that's OK, I don't have a real reason for keeping the last 3 versions of go, the last 2 is enough.

@zavla zavla requested a review from fclairamb March 29, 2020 12:52
@fclairamb
Copy link
Owner

@zavla there's still written qouteDoubling instead of quoteDoubling everywhere.

Copy link
Owner

@fclairamb fclairamb left a comment

Choose a reason for hiding this comment

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

LGTM

@probot-auto-merge probot-auto-merge bot merged commit bc791b0 into fclairamb:master Mar 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants