Skip to content
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

feat: optimize artisan check #238

Merged
merged 10 commits into from
Jul 15, 2023
Merged

feat: optimize artisan check #238

merged 10 commits into from
Jul 15, 2023

Conversation

devhaozi
Copy link
Member

@devhaozi devhaozi commented Jul 10, 2023

📑 Description

对昨天#237 的补充

✅ Checks

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed

ℹ Additional Information

@devhaozi devhaozi requested a review from hwbrzzl July 10, 2023 10:31
@devhaozi devhaozi self-assigned this Jul 10, 2023
crypt/aes.go Show resolved Hide resolved
route/gin.go Show resolved Hide resolved
@devhaozi
Copy link
Member Author

Lint会在发现问题时输出一堆毫无意义的信息:
image
参考golangci/golangci-lint-action#677,禁用缓存似乎可以解决。

@devhaozi
Copy link
Member Author

修复了:
image

@devhaozi devhaozi enabled auto-merge (squash) July 10, 2023 10:44
@devhaozi devhaozi requested a review from hwbrzzl July 12, 2023 05:04
assert.Nil(t, file.Remove("config.conf"))

mockConfig.AssertExpectations(t)
}

Choose a reason for hiding this comment

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

Bug risks:

  • The code modifies the global variable support.EnvPath without restoring its original value after the test. This can affect other tests that rely on the default value of support.EnvPath.

Improvement suggestions:

  • Instead of modifying the global variable support.EnvPath, it would be better to use a local variable within the test function to specify the custom environment file path.
  • The code uses assert statements for checking conditions and error handling. Consider using testing functions like Errorf or FailNow to provide more informative failure messages when assertions fail.
  • There is an opportunity to improve code readability by breaking down the second test function into smaller, more focused test cases.

Copy link
Member Author

Choose a reason for hiding this comment

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

Bug risks:

  • The code modifies the global variable support.EnvPath without restoring its original value after the test. This can affect other tests that rely on the default value of support.EnvPath.

Improvement suggestions:

  • Instead of modifying the global variable support.EnvPath, it would be better to use a local variable within the test function to specify the custom environment file path.
  • The code uses assert statements for checking conditions and error handling. Consider using testing functions like Errorf or FailNow to provide more informative failure messages when assertions fail.
  • There is an opportunity to improve code readability by breaking down the second test function into smaller, more focused test cases.

Good idea.

@devhaozi devhaozi requested a review from hwbrzzl July 12, 2023 11:11
@devhaozi
Copy link
Member Author

@hwbrzzl Codecov 似乎有问题,Token 404

image

@devhaozi
Copy link
Member Author

cc @hwbrzzl

Copy link
Contributor

@hwbrzzl hwbrzzl left a comment

Choose a reason for hiding this comment

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

Good catch, LGTM 👍

@devhaozi devhaozi merged commit 5b0c133 into master Jul 15, 2023
10 checks passed
@devhaozi devhaozi deleted the haozi-env branch July 15, 2023 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants