-
Notifications
You must be signed in to change notification settings - Fork 34
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: add linters #402
feat: add linters #402
Conversation
WalkthroughThe recent updates encompass a variety of improvements and additions across multiple files. These include enhanced configuration settings, new GitHub Actions workflows for linting, updated contribution guidelines, refined Makefile targets, and better error handling. Additionally, there are numerous import statement reorders and minor code adjustments to improve readability, maintainability, and functionality. The changes also address specific linting requirements and error handling improvements. Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 3
Outside diff range and nitpick comments (1)
pkg/instance/helper.go (1)
Line range hint
398-398
: Specify a more restrictive IP address instead of "0.0.0.0" to enhance security.- return nil, ErrSettingBitTwisterEnv.Wrap(err) + if err := bt.SetEnvironmentVariable("SERVE_ADDR", fmt.Sprintf("127.0.0.1:%d", i.BitTwister.Port())); err != nil { + return nil, ErrSettingBitTwisterEnv.Wrap(err) + }
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (35)
- .env (1 hunks)
- .github/workflows/knuu_testing.yml (1 hunks)
- .github/workflows/lint.yml (1 hunks)
- .gitignore (1 hunks)
- .golangci.yml (1 hunks)
- .markdownlint.yaml (1 hunks)
- .yamllint.yml (1 hunks)
- CONTRIBUTING.md (1 hunks)
- Makefile (1 hunks)
- README.md (1 hunks)
- docs/architecture/adr-001-redesign-knuu-async-approach.md (1 hunks)
- e2e/basic/file_test_image_cached_test.go (1 hunks)
- e2e/basic/files_to_volumes_cm_test.go (2 hunks)
- e2e/basic/folder_test.go (1 hunks)
- e2e/basic/folder_test_image_cached_test.go (1 hunks)
- e2e/basic/main_test.go (1 hunks)
- e2e/basic/probe_test.go (1 hunks)
- e2e/basic/rbac_test.go (1 hunks)
- pkg/builder/docker/docker.go (1 hunks)
- pkg/builder/kaniko/kaniko.go (1 hunks)
- pkg/builder/kaniko/kaniko_test.go (2 hunks)
- pkg/builder/kaniko/utils_test.go (1 hunks)
- pkg/container/docker.go (1 hunks)
- pkg/instance/bittwister.go (1 hunks)
- pkg/instance/helper.go (1 hunks)
- pkg/instance/instance.go (1 hunks)
- pkg/k8s/k8s_custom_resource.go (1 hunks)
- pkg/k8s/k8s_pod.go (2 hunks)
- pkg/k8s/types.go (4 hunks)
- pkg/knuu/instance_old.go (1 hunks)
- pkg/knuu/knuu_test.go (1 hunks)
- pkg/minio/minio.go (1 hunks)
- pkg/preloader/preloader.go (1 hunks)
- pkg/system/dependencies.go (1 hunks)
- pkg/traefik/traefik.go (2 hunks)
Files skipped from review due to trivial changes (22)
- .env
- .github/workflows/knuu_testing.yml
- .github/workflows/lint.yml
- .gitignore
- .golangci.yml
- .markdownlint.yaml
- .yamllint.yml
- README.md
- docs/architecture/adr-001-redesign-knuu-async-approach.md
- e2e/basic/file_test_image_cached_test.go
- e2e/basic/files_to_volumes_cm_test.go
- e2e/basic/folder_test.go
- e2e/basic/main_test.go
- e2e/basic/rbac_test.go
- pkg/builder/docker/docker.go
- pkg/instance/bittwister.go
- pkg/knuu/instance_old.go
- pkg/knuu/knuu_test.go
- pkg/minio/minio.go
- pkg/preloader/preloader.go
- pkg/system/dependencies.go
- pkg/traefik/traefik.go
Additional comments not posted (24)
Makefile (1)
5-9
: The additions to the Makefile are well-structured and enhance the build process. Good use of.PHONY
to ensure that targets are always executed.Also applies to: 11-16, 18-23, 25-30, 32-42, 44-50, 52-56, 58-59
e2e/basic/folder_test_image_cached_test.go (1)
40-40
: Switching tot.Errorf
allows the test to continue and report further errors, which is preferable in most testing scenarios.pkg/builder/kaniko/utils_test.go (1)
25-25
: Changing permissions to0600
for directories and files in tests is a secure practice, ensuring restricted access.Also applies to: 28-28
e2e/basic/probe_test.go (1)
Line range hint
1-1
: No changes detected in this file for review.pkg/builder/kaniko/kaniko_test.go (3)
Line range hint
16-50
: The changes in theTestKanikoBuilder
function look good and cover multiple scenarios effectively.
16-16
: The import of thebuilder
package is correctly placed and used within the test functions.
16-16
: The import of thebuilder
package is correctly placed and used within the test functions.pkg/k8s/types.go (2)
20-20
: The update to usecorev1.ConfigMap
in theCreateConfigMap
function signature is correct and aligns with the updated imports.
22-29
: The updates to usecorev1
types in theCreateDaemonSet
,CreateService
,PatchService
, andUpdateDaemonSet
function signatures are correct and ensure consistency with the updated imports.Also applies to: 63-63, 75-75
pkg/container/docker.go (1)
249-255
: TherunCommand
function correctly captures stdout and stderr, and it properly formats the error message to include these details.pkg/builder/kaniko/kaniko.go (2)
16-18
: The imports in the file are correctly organized and relevant to the functionalities used in the Kaniko builder implementation.
16-18
: The Kaniko builder implementation is comprehensive, covering job preparation, monitoring, and cleanup effectively. Error handling and logging are appropriately implemented.pkg/k8s/k8s_pod.go (1)
Line range hint
109-122
: TheReplacePodWithGracePeriod
function is well-implemented with robust error handling and detailed logging. The use of labels for loop control improves readability and control flow.pkg/instance/helper.go (11)
17-17
: Ensure the new importgithub.com/celestiaorg/knuu/pkg/k8s
is utilized effectively throughout the file.
18-18
: Good use of conditional checks for sidecar instances in service deployment logic.
18-18
: Proper implementation of service destruction logic.
18-18
: Well-structured function for pod deployment with comprehensive error handling.
18-18
: Effective management of pod and associated resource deletion with grace period handling.
18-18
: Correct implementation of volume deployment logic.
18-18
: Proper implementation of volume destruction logic.
18-18
: Well-implemented function for deploying files as configmaps.
18-18
: Effective implementation of file destruction logic.
18-18
: Comprehensive resource deployment function with robust error handling.
18-18
: Comprehensive resource destruction function with robust error handling.
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.
Actionable comments posted: 0
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.
Actionable comments posted: 3
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- .github/workflows/lint.yml (1 hunks)
- CONTRIBUTING.md (1 hunks)
- Makefile (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- .github/workflows/lint.yml
- Makefile
Additional Context Used
LanguageTool (3)
CONTRIBUTING.md (3)
Near line 37: Did you mean “either”?
Context: ...a feature, it should not have bug fixes too. This is to enable the maintainers revi...
Rule ID: NOT_TOO_EITHER
Near line 37: Possible missing preposition found.
Context: ... too. This is to enable the maintainers review your PR efficiently. 1. Commit the cha...
Rule ID: AI_HYDRA_LEO_MISSING_TO
Near line 58: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 2127 characters long)
Context: ...with thefix:
prefix. Congratulations! You have successfully made a Pull Reque...
Rule ID: EN_EXCESSIVE_EXCLAMATION
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! Thanks!
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 🚀
left a question.
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- e2e/basic/folder_test.go (1 hunks)
- pkg/builder/kaniko/utils_test.go (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- e2e/basic/folder_test.go
- pkg/builder/kaniko/utils_test.go
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!
Overview
Closes: #372
This PR:
NOTE: the markdownlint link checker is failing on linking to /e2e because there is nothing in the directory except the subdirectory, it actually 404s. This PR will actually resolve that if it gets merged: #404
Summary by CodeRabbit
New Features
Documentation
Chores
.gitignore
file for specific files and directories.Bug Fixes
t.Fatalf
tot.Errorf
.WaitInstanceIsRunning
method.