-
Notifications
You must be signed in to change notification settings - Fork 13
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: setup for the project #172
fix: setup for the project #172
Conversation
- Bump the strmangle dependency from `v0.0.4` to `v0.0.6` - Remove the `sqlboiler` v3, only v4 should be used in the project - Update README for clean instructions on the `seeding` the entities - Update Dockerfile - The docker build will now be quicker due to the caching of dependency download
WalkthroughThe recent updates focus on optimizing the Dockerfile by reordering the steps for dependency management and enhancing the clarity and usability of the Changes
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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: 4
Outside diff range and nitpick comments (3)
Dockerfile (2)
Line range hint
22-25
: Consider consolidating multipleRUN
instructions.Multiple consecutive
RUN
instructions can be combined to reduce the number of layers in the Docker image.- RUN go run ./cmd/seeder/main.go - RUN go build -o ./output/server ./cmd/server/main.go - RUN go build -o ./output/migrations ./cmd/migrations/main.go - RUN go build -o ./output/seeder ./cmd/seeder/exec/seed.go + RUN go run ./cmd/seeder/main.go && \ + go build -o ./output/server ./cmd/server/main.go && \ + go build -o ./output/migrations ./cmd/migrations/main.go && \ + go build -o ./output/seeder ./cmd/seeder/exec/seed.goTools
Hadolint
[warning] 2-2: Pin versions in apk add. Instead of
apk add <package>
useapk add <package>=<version>
(DL3018)
[info] 2-2: Use the
--no-cache
switch to avoid the need to use--update
and remove/var/cache/apk/*
when done installing packages (DL3019)
[error] 7-7: Use COPY instead of ADD for files and folders (DL3020)
[error] 8-8: Use COPY instead of ADD for files and folders (DL3020)
[error] 12-12: Use COPY instead of ADD for files and folders (DL3020)
Line range hint
28-28
: Pin the Alpine version explicitly.Using
latest
for the Alpine version can lead to unpredictable builds if the image updates.- FROM alpine:latest + FROM alpine:<specific-version>Tools
Hadolint
[warning] 2-2: Pin versions in apk add. Instead of
apk add <package>
useapk add <package>=<version>
(DL3018)
[info] 2-2: Use the
--no-cache
switch to avoid the need to use--update
and remove/var/cache/apk/*
when done installing packages (DL3019)
[error] 7-7: Use COPY instead of ADD for files and folders (DL3020)
[error] 8-8: Use COPY instead of ADD for files and folders (DL3020)
[error] 12-12: Use COPY instead of ADD for files and folders (DL3020)
README.MD (1)
Line range hint
1-1
: Add alternate text to images.Images should have alternate text to improve accessibility and SEO.
- <img src="https://github.com/wednesday-solutions/go-template/blob/master/golang_template_github.svg" width="440" height="480" /> + <img src="https://github.com/wednesday-solutions/go-template/blob/master/golang_template_github.svg" width="440" height="480" alt="Go Template Logo" />Also applies to: 5-5, 24-24, 27-27
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
go.sum
is excluded by!**/*.sum
Files selected for processing (5)
- Dockerfile (1 hunks)
- README.MD (10 hunks)
- go.mod (2 hunks)
- scripts/setup-pre-commit.sh (1 hunks)
- tools.go (1 hunks)
Files skipped from review due to trivial changes (1)
- go.mod
Additional context used
Hadolint
Dockerfile
[warning] 2-2: Pin versions in apk add. Instead of
apk add <package>
useapk add <package>=<version>
(DL3018)
[info] 2-2: Use the
--no-cache
switch to avoid the need to use--update
and remove/var/cache/apk/*
when done installing packages (DL3019)
[error] 7-7: Use COPY instead of ADD for files and folders (DL3020)
[error] 8-8: Use COPY instead of ADD for files and folders (DL3020)
[error] 12-12: Use COPY instead of ADD for files and folders (DL3020)
[info] 22-22: Multiple consecutive
RUN
instructions. Consider consolidation. (DL3059)
[info] 23-23: Multiple consecutive
RUN
instructions. Consider consolidation. (DL3059)
[info] 24-24: Multiple consecutive
RUN
instructions. Consider consolidation. (DL3059)
[info] 25-25: Multiple consecutive
RUN
instructions. Consider consolidation. (DL3059)
[warning] 28-28: Using latest is prone to errors if the image will ever update. Pin the version explicitly to a release tag (DL3007)
[warning] 29-29: Pin versions in apk add. Instead of
apk add <package>
useapk add <package>=<version>
(DL3018)
[info] 30-30: Multiple consecutive
RUN
instructions. Consider consolidation. (DL3059)
[warning] 30-30: Pin versions in apk add. Instead of
apk add <package>
useapk add <package>=<version>
(DL3018)
LanguageTool
README.MD
[style] ~33-~33: Consider a different adverb to strengthen your wording. (ALWAYS_CONSTANTLY)
Context: ...4"> --- We’re always looking for people who value their work...
[style] ~98-~98: Consider shortening or rephrasing this to strengthen your wording. (MAKE_CHANGES)
Context: ... the using docker-compose everytime you make changes to it # Setting up database (postgres) -...
[grammar] ~252-~252: Did you mean “be generated”? (WILL_BASED_ON)
Context: ...kg/api/api.go) ## Schema - Schema can generated or altered manually Take a look at t...
[grammar] ~275-~275: The word ‘container’ is a noun or an adjective. A verb or adverb is missing or misspelled here, or maybe a comma is missing. (PRP_MD_NN)
Context: ... and profiles. Application name should container only lowercase letters. No hyphens and ...
Markdownlint
README.MD
31-31: Expected: ___; Actual: --- (MD035, hr-style)
Horizontal rule style
38-38: Expected: ___; Actual: --- (MD035, hr-style)
Horizontal rule style
69-69: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
77-77: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
84-84: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
131-131: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
137-137: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
143-143: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
149-149: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
217-217: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
225-225: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
236-236: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
277-277: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
285-285: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
302-302: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
317-317: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
323-323: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
331-331: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
339-339: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
345-345: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
1-1: null (MD045, no-alt-text)
Images should have alternate text (alt text)
5-5: null (MD045, no-alt-text)
Images should have alternate text (alt text)
24-24: null (MD045, no-alt-text)
Images should have alternate text (alt text)
27-27: null (MD045, no-alt-text)
Images should have alternate text (alt text)
Additional comments not posted (3)
tools.go (1)
11-11
: Approved the update tosqlboiler
import.The update to use
github.com/volatiletech/sqlboiler/v4
aligns with the project's decision to standardize on version 4 ofsqlboiler
.scripts/setup-pre-commit.sh (1)
16-16
: Approved the addition ofgo-commitlinter
.Installing
go-commitlinter
and adding it to the commit message hook enhances commit message quality control, which is beneficial for maintaining project standards.Dockerfile (1)
4-4
: ChangeRUN mkdir /app
toWORKDIR /app
.This is a cleaner and more idiomatic way to set the working directory in a Dockerfile.
- Reformat the `README.md`
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: 1
Outside diff range and nitpick comments (1)
Dockerfile (1)
Line range hint
28-28
: Pin the version of the Alpine base image.Using
latest
is prone to errors if the image updates. Pinning the version explicitly to a release tag ensures consistency and predictability.- FROM alpine:latest + FROM alpine:3.19Tools
Hadolint
[warning] 2-2: Pin versions in apk add. Instead of
apk add <package>
useapk add <package>=<version>
(DL3018)
[info] 2-2: Use the
--no-cache
switch to avoid the need to use--update
and remove/var/cache/apk/*
when done installing packages (DL3019)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- Dockerfile (1 hunks)
- README.MD (9 hunks)
Additional context used
Hadolint
Dockerfile
[warning] 2-2: Pin versions in apk add. Instead of
apk add <package>
useapk add <package>=<version>
(DL3018)
[info] 2-2: Use the
--no-cache
switch to avoid the need to use--update
and remove/var/cache/apk/*
when done installing packages (DL3019)
[info] 22-22: Multiple consecutive
RUN
instructions. Consider consolidation. (DL3059)
[info] 23-23: Multiple consecutive
RUN
instructions. Consider consolidation. (DL3059)
[info] 24-24: Multiple consecutive
RUN
instructions. Consider consolidation. (DL3059)
[info] 25-25: Multiple consecutive
RUN
instructions. Consider consolidation. (DL3059)
[warning] 28-28: Using latest is prone to errors if the image will ever update. Pin the version explicitly to a release tag (DL3007)
[warning] 29-29: Pin versions in apk add. Instead of
apk add <package>
useapk add <package>=<version>
(DL3018)
[info] 30-30: Multiple consecutive
RUN
instructions. Consider consolidation. (DL3059)
[warning] 30-30: Pin versions in apk add. Instead of
apk add <package>
useapk add <package>=<version>
(DL3018)
LanguageTool
README.MD
[style] ~32-~32: Consider a different adverb to strengthen your wording. (ALWAYS_CONSTANTLY)
Context: ..."> --- We’re always looking for people who value their work...
[uncategorized] ~81-~81: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...boiler-psql@latestFor Go 1.18 and above install the sql-migrate using
bash ...
[style] ~97-~97: Consider shortening or rephrasing this to strengthen your wording. (MAKE_CHANGES)
Context: ... the using docker-compose everytime you make changes to it # Setting up database (postgres) -...
[grammar] ~251-~251: Did you mean “be generated”? (WILL_BASED_ON)
Context: ...kg/api/api.go) ## Schema - Schema can generated or altered manually Take a look at t...
[grammar] ~274-~274: The word ‘container’ is a noun or an adjective. A verb or adverb is missing or misspelled here, or maybe a comma is missing. (PRP_MD_NN)
Context: ... and profiles. Application name should container only lowercase letters. No hyphens and ...
Markdownlint
README.MD
37-37: Expected: ___; Actual: --- (MD035, hr-style)
Horizontal rule style
1-1: null (MD045, no-alt-text)
Images should have alternate text (alt text)
5-5: null (MD045, no-alt-text)
Images should have alternate text (alt text)
24-24: null (MD045, no-alt-text)
Images should have alternate text (alt text)
27-27: null (MD045, no-alt-text)
Images should have alternate text (alt text)
Additional comments not posted (4)
Dockerfile (2)
7-8
: ReplaceADD
withCOPY
for files and folders.As previously noted by the bot and supported by Docker best practices, using
COPY
for files and directories is recommended overADD
unless the functionality ofADD
is required.
Line range hint
29-29
: Pin versions inapk add
commands.It's crucial to pin package versions to avoid unexpected changes when newer versions are released. This practice ensures reproducibility and stability of Docker builds.
Also applies to: 30-30
Tools
Hadolint
[warning] 2-2: Pin versions in apk add. Instead of
apk add <package>
useapk add <package>=<version>
(DL3018)
[info] 2-2: Use the
--no-cache
switch to avoid the need to use--update
and remove/var/cache/apk/*
when done installing packages (DL3019)README.MD (2)
30-30
: Adjust horizontal rule styles for consistency.Consistent use of horizontal rule styles enhances readability and maintains a uniform appearance throughout the document.
68-71
: Ensure the installation instructions are up-to-date.Given the PR's objective to remove
sqlboiler
v3 and focus on v4, ensure the installation commands reflect these changes accurately to avoid confusion.Also applies to: 76-79, 83-85
Verification successful
Ensure the installation instructions are up-to-date.
The installation commands in the
README.MD
file correctly referencesqlboiler v4
. Here are the relevant lines:
- Line 77:
go install github.com/volatiletech/sqlboiler/v4@latest
- Line 78:
go install github.com/volatiletech/sqlboiler/v4/drivers/sqlboiler-psql@latest
These lines confirm that the instructions have been updated to reflect
sqlboiler v4
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct installation commands for sqlboiler v4. # Test: Search for sqlboiler installation commands. Expect: Only commands for sqlboiler v4. rg --type markdown $'go install github.com/volatiletech/sqlboiler/v4@latest'Length of output: 226
Script:
#!/bin/bash # Description: Verify the correct installation commands for sqlboiler. # Test: Search for sqlboiler installation commands in markdown files. rg --type markdown 'sqlboiler'Length of output: 179
Script:
#!/bin/bash # Description: Debug why ripgrep is not searching any files. # Test: Run ripgrep with debug flag to identify why files are being skipped. rg --type markdown 'sqlboiler' --debugLength of output: 25447
Script:
#!/bin/bash # Description: Verify the correct installation commands for sqlboiler. # Test: Search for sqlboiler installation commands in markdown files, ignoring any ignore rules. rg --type markdown 'sqlboiler' --no-ignoreLength of output: 191
Script:
#!/bin/bash # Description: Verify the correct installation commands for sqlboiler using fd and grep. # Find all markdown files and search for sqlboiler installation commands. fd -e md -x grep -Hn 'sqlboiler' {}Length of output: 512
@coderabbitai review |
Actions performedReview triggered.
|
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: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- Dockerfile (2 hunks)
Additional context used
Hadolint
Dockerfile
[info] 2-2: Use the
--no-cache
switch to avoid the need to use--update
and remove/var/cache/apk/*
when done installing packages (DL3019)
[warning] 2-2: Pin versions in apk add. Instead of
apk add <package>
useapk add <package>=<version>
(DL3018)
[warning] 28-28: Using latest is prone to errors if the image will ever update. Pin the version explicitly to a release tag (DL3007)
[warning] 29-29: Pin versions in apk add. Instead of
apk add <package>
useapk add <package>=<version>
(DL3018)
[info] 30-30: Multiple consecutive
RUN
instructions. Consider consolidation. (DL3059)
[warning] 30-30: Pin versions in apk add. Instead of
apk add <package>
useapk add <package>=<version>
(DL3018)
Additional comments not posted (3)
Dockerfile (3)
3-3
: Good use ofWORKDIR
to simplify directory handling.Using
WORKDIR
instead ofmkdir
is cleaner and adheres to Docker best practices.
7-8
: Optimize dependency management by copyinggo.mod
andgo.sum
early.This change effectively uses Docker's caching mechanism to speed up builds. However, consider using
COPY
instead ofADD
for all file copying operations to align with Docker best practices.
22-25
: Excellent consolidation ofRUN
commands to minimize Docker layers.Consolidating these commands into fewer
RUN
statements is a great practice to reduce the number of layers, which can lead to smaller Docker image sizes and faster builds.
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
🧹 Outside diff range and nitpick comments (3)
README.MD (3)
68-71
: LGTM: Improved code block formattingThe changes to the code block are excellent:
- Adding the
bash
language specification improves syntax highlighting and readability.- Using line continuation makes the command more readable and easier to maintain.
These updates align with best practices for documentation.
Consider adding a backslash () at the end of line 70 for consistency in line continuation:
go get -v github.com/rubenv/sql-migrate/... \ -github.com/volatiletech/sqlboiler \ +github.com/volatiletech/sqlboiler \ github.com/99designs/gqlgen
97-98
: LGTM: Improved clarity in .env.base instructionsThe updated note provides clearer instructions about the .env.base file and emphasizes the need to rebuild using docker-compose after making changes. This improvement helps prevent potential setup issues for users.
Consider rephrasing the second part of the note for better clarity:
-**NOTE:** Please do not delete `.env.base` file of the project and rebuild the using docker-compose everytime you -make changes to it +**NOTE:** Please do not delete the `.env.base` file of the project. Rebuild using docker-compose every time you +make changes to this file.This rephrasing separates the two instructions more clearly and fixes a minor grammatical issue.
🧰 Tools
🪛 LanguageTool
[style] ~97-~97: Consider shortening or rephrasing this to strengthen your wording.
Context: ... the using docker-compose everytime you make changes to it # Setting up database (postgres) -...(MAKE_CHANGES)
216-217
: LGTM: Improved GraphQL and seeding instructionsThe changes in the GraphQL and seeding sections are excellent:
- Adding
bash
language specifications to code blocks improves syntax highlighting and readability.- The new command for seeding provides a more complete set of instructions for database setup.
These updates enhance the overall quality of the documentation and make it easier for users to follow the setup process.
Consider adding a brief explanation of the difference between the two seeding commands:
go run cmd/seeder/main.go ## to build the execs for seeding go run cmd/seeder/exec/seed.go ## to seed + +# The first command builds the seeding executables, while the second command actually performs the seeding operation.This additional explanation will help users understand the purpose of each command in the seeding process.
Also applies to: 224-226, 235-236
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- README.MD (9 hunks)
🧰 Additional context used
🪛 LanguageTool
README.MD
[style] ~32-~32: Consider a different adverb to strengthen your wording.
Context: ..."> --- We’re always looking for people who value their work...(ALWAYS_CONSTANTLY)
[style] ~97-~97: Consider shortening or rephrasing this to strengthen your wording.
Context: ... the using docker-compose everytime you make changes to it # Setting up database (postgres) -...(MAKE_CHANGES)
[grammar] ~251-~251: Did you mean “be generated”?
Context: ...kg/api/api.go) ## Schema - Schema can generated or altered manually Take a look at t...(WILL_BASED_ON)
🪛 Markdownlint
README.MD
24-24: null
Images should have alternate text (alt text)(MD045, no-alt-text)
27-27: null
Images should have alternate text (alt text)(MD045, no-alt-text)
🔇 Additional comments (8)
README.MD (8)
4-6
: LGTM: URL update for better trackingThe URL update from "gthb" to "github" in the Wednesday Solutions link is correct. This change ensures proper tracking of traffic from GitHub and improves the overall accuracy of the link.
🧰 Tools
🪛 Markdownlint
5-5: null
Images should have alternate text (alt text)(MD045, no-alt-text)
30-30
: Great job implementing the suggested change!The horizontal rule style has been updated from "---" to "___", which aligns with the previous review comment for consistency. This change improves the overall document consistency.
32-33
: LGTM: Consistent URL update for hiring linkThe URL update from "gthb" to "github" in the "We are hiring!" link is correct and consistent with the earlier changes. This ensures accurate tracking across all links in the document.
🧰 Tools
🪛 LanguageTool
[style] ~32-~32: Consider a different adverb to strengthen your wording.
Context: ..."> --- We’re always looking for people who value their work...(ALWAYS_CONSTANTLY)
106-108
: LGTM: Improved database setup instructionsThe updates to the database setup instructions are excellent:
- The instructions are now presented as a numbered list, improving readability.
- The commands are properly formatted as inline code blocks.
These changes make the setup process clearer and easier to follow, which will help users set up the database correctly.
199-200
: LGTM: Added .env.base to file structureThe addition of the .env.base file to the file structure is a good improvement. It provides a more complete picture of the project layout and reinforces the importance of this file, which was mentioned earlier in the setup instructions. This change helps users better understand the project's file organization.
284-286
: LGTM: Enhanced infrastructure configuration examplesThe additions to the Infrastructure section are valuable improvements:
- YAML code blocks with language specifications enhance readability and syntax highlighting.
- The examples clearly demonstrate how to configure environment variables and integrate with an S3 bucket.
These changes provide users with concrete examples of infrastructure configuration, making it easier to set up the project correctly.
Also applies to: 293-294
313-314
: LGTM: Consistent code block improvementsThe changes in the deployment, testing, and mock generation sections are excellent:
- Language specifications (
bash
) have been added to all code blocks.- This improvement enhances readability and ensures consistent syntax highlighting throughout the document.
- The uniformity in code block formatting contributes to a more professional and polished documentation.
These updates make it easier for users to identify and execute commands, improving the overall user experience.
Also applies to: 319-320, 327-328, 335-336, 341-342
Line range hint
1-353
: Overall excellent improvements to the READMEThe changes made to this README file significantly enhance its quality and usability:
- Consistent URL updates improve tracking and analytics.
- Uniform formatting of code blocks with language specifications enhances readability.
- Clarified instructions for environment setup and database configuration.
- Added examples for infrastructure configuration.
- Improved overall document structure and consistency.
These updates collectively make the documentation more professional, easier to follow, and more helpful for users setting up and working with the project. Great job on these improvements!
🧰 Tools
🪛 LanguageTool
[grammar] ~274-~274: The word ‘container’ is a noun or an adjective. A verb or adverb is missing or misspelled here, or maybe a comma is missing.
Context: ... and profiles. Application name should container only lowercase letters. No hyphens and ...(PRP_MD_NN)
New PR has been raised for the same issue |
Ticket Link
Related Links
Description
v0.0.4
tov0.0.6
sqlboiler
v3, only v4 should be used in the projectseeding
the entitiesSteps to Reproduce / Test
make tests
Request
Response
Summary by CodeRabbit
Chores
Documentation
README.md
content for improved clarity and flow.