-
Notifications
You must be signed in to change notification settings - Fork 500
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.
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
fix make lint
#495
fix make lint
#495
Conversation
@@ -21,6 +20,8 @@ warningCode = 0 | |||
[rule.modifies-parameter] | |||
|
|||
# Add these once issues are fixed | |||
[rule.var-naming] | |||
severity = "warning" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A lot of places violates rule.var-naming
, it makes the PR too complicated to fix all.
I decide to change the error level of it to "warning" temporarily. We can fix it in a separate PR.
/run-e2e-tests |
"os" | ||
"os/user" | ||
"path/filepath" | ||
"sync" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imports changed because the code is formatted by goimports
tool in my editor. It automatically puts the standard packages at the front of the import list.
@@ -42,7 +42,7 @@ func serve(w http.ResponseWriter, r *http.Request, admit admitFunc) { | |||
goto returnData | |||
} | |||
} else { | |||
err := errors.New("request body is nil!") | |||
err := errors.New("request body is nil") | |||
responseAdmissionReview.Response = toAdmissionResponse(err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is required by rule.error-strings
.
@@ -10,6 +10,7 @@ | |||
// distributed under the License is distributed on an "AS IS" BASIS, | |||
// See the License for the specific language governing permissions and | |||
// limitations under the License. | |||
|
|||
package ops |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is required by [rule.package-comments]
.
@@ -9,6 +9,7 @@ import ( | |||
|
|||
"github.com/pingcap/tidb-operator/tests/slack" | |||
|
|||
// To register MySQL driver | |||
_ "github.com/go-sql-driver/mysql" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is required by [rule.blank-imports]
.
It seems that revive does not check test files. I'm looking at how to fix. |
/run-e2e-tests |
- fix `make lint` - rule.var-naming to warning, change it to error in future
/run-e2e-tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What problem does this PR solve?
fixes #481
What is changed and how it works?
Note that a lot of places violates
rule.var-naming
, it makes the PR too complicated to fix all.I decide to change the error level of it to "warning" temporarily. We can fix it in a separate PR.
Check List
Tests
Unit test
Does this PR introduce a user-facing change?: