Add Clojure code #127
Add Clojure code #127
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis change introduces a complete Clojure starter and solution template for the "Build Your Own HTTP Server" challenge. It adds build scripts, configuration files, Docker support, documentation, and a simple TCP server implementation. The structure is mirrored across starter, compiled, and solution directories, enabling both local and CodeCrafters platform execution. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CompileScript
participant BuildScript
participant JavaRuntime
User->>CompileScript: Run compile.sh
CompileScript->>BuildScript: clj -T:build
BuildScript->>BuildScript: clean, compile, uberjar
BuildScript-->>CompileScript: Build complete
User->>RunScript: Run run.sh
RunScript->>JavaRuntime: java -jar target.jar [args]
JavaRuntime->>JavaRuntime: Start http-server.core/-main
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (4)
compiled_starters/clojure/.codecrafters/run.sh (1)
1-11: Duplicate run script.This file is identical to other
.codecrafters/run.shscripts.solutions/clojure/01-at4/code/.codecrafters/run.sh (1)
1-11: Duplicate run script.This file matches the run script in both starter templates and compiled starters.
compiled_starters/clojure/src/http_server/core.clj (1)
1-24: Same issues as starter template versionThis file is identical to
starter_templates/clojure/code/src/http_server/core.cljand has the same issues: unused import, resource leaks, and incomplete connection handling that could cause test hangs.compiled_starters/clojure/build.clj (1)
1-23: 🛠️ Refactor suggestionAddress code duplication between build scripts.
This file is identical to
starter_templates/clojure/code/build.clj. Having duplicate build scripts creates maintenance overhead and increases the risk of inconsistencies.Consider one of these approaches:
- Use symbolic links if the files should always be identical
- Extract common build logic to a shared location
- Use a build script generator if they need slight variations
The same technical issues from the starter template version also apply here:
- Hard-coded temporary paths may not be portable
- Missing error handling for build operations
- Missing newline at end of file
- Need to verify the
http-server.corenamespace exists and compiles correctly
🧹 Nitpick comments (12)
compiled_starters/clojure/deps.edn (1)
2-3: Consider including test and resource paths.
Currently only"src"is in:paths. If you plan to add tests or include resource files, consider adding"test"and/or"resources"here so they’re on the classpath.solutions/clojure/01-at4/code/codecrafters.yml (1)
5-5: Verbose debug logs caution.
Havingdebug: trueby default may flood CI output and obscure test failures. Consider defaulting it tofalseor making it configurable via an environment variable.compiled_starters/clojure/.gitattributes (1)
1-1: Restrict line ending normalization.
Using* text=autoapplies to all files, including binaries. Consider targeting only source files (e.g.,*.clj text=auto) to avoid unintended conversions.compiled_starters/clojure/codecrafters.yml (1)
5-5: Default debug mode caution.
Debug logs are enabled, which may overwhelm the test harness. Consider disabling by default or gating it behind an environment flag.solutions/clojure/01-at4/explanation.md (1)
12-16: Specify language for fenced code block
Add a language identifier to satisfy Markdown lint rules for the shell snippet.- ``` + ```bash🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
12-12: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
solutions/clojure/01-at4/code/your_program.sh (1)
1-24: Add missing newline at end of file.The script logic is correct and well-documented, but it's missing a newline at the end of the file which could cause issues with some tools.
Apply this diff to fix the formatting:
exec java -jar /tmp/codecrafters-build-http-server-clojure/target.jar "$@" +starter_templates/clojure/code/src/http_server/core.clj (1)
3-3: Remove unused importThe
clojure.java.ionamespace is imported but never used in the code.- (:require - [clojure.java.io :as io])compiled_starters/clojure/README.md (1)
7-7: Fix grammar: use "an" before vowel soundThe static analysis tool correctly identified a grammar issue.
-that is capable of serving multiple clients. +protocol that powers the web. In this challenge, you'll build an HTTP/1.1 server🧰 Tools
🪛 LanguageTool
[misspelling] ~7-~7: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...he web. In this challenge, you'll build a HTTP/1.1 server that is capable of serv...(EN_A_VS_AN)
solutions/clojure/01-at4/code/README.md (1)
7-7: Minor grammar fix: Use "an" before vowel sounds.Replace "a HTTP" with "an HTTP" for correct grammar.
-protocol that powers the web. In this challenge, you'll build a HTTP/1.1 server +protocol that powers the web. In this challenge, you'll build an HTTP/1.1 server🧰 Tools
🪛 LanguageTool
[misspelling] ~7-~7: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...he web. In this challenge, you'll build a HTTP/1.1 server that is capable of serv...(EN_A_VS_AN)
starter_templates/clojure/code/build.clj (3)
5-8: Consider making build paths configurable.The hard-coded temporary paths may not be portable across different operating systems or environments. Consider using system properties or environment variables for better portability.
-(def class-dir "/tmp/codecrafters-build-http-server-clojure/classes") -(def uber-file "/tmp/codecrafters-build-http-server-clojure/target.jar") +(def build-dir (or (System/getProperty "codecrafters.build.dir") + (str (System/getProperty "java.io.tmpdir") "/codecrafters-build-http-server-clojure"))) +(def class-dir (str build-dir "/classes")) +(def uber-file (str build-dir "/target.jar"))
10-11: Add error handling for clean operation.The clean function should handle cases where the directory doesn't exist or deletion fails to provide clearer error messages during the build process.
(defn clean [_] - (b/delete {:path "/tmp/codecrafters-build-http-server-clojure"})) + (try + (b/delete {:path "/tmp/codecrafters-build-http-server-clojure"}) + (catch Exception e + (println "Warning: Could not clean build directory:" (.getMessage e)))))
23-23: Add missing newline at end of file.The file is missing a newline character at the end, which can cause issues with some tools and is considered best practice.
:main 'http-server.core})) +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
compiled_starters/clojure/.codecrafters/compile.sh(1 hunks)compiled_starters/clojure/.codecrafters/run.sh(1 hunks)compiled_starters/clojure/.gitattributes(1 hunks)compiled_starters/clojure/.gitignore(1 hunks)compiled_starters/clojure/README.md(1 hunks)compiled_starters/clojure/build.clj(1 hunks)compiled_starters/clojure/codecrafters.yml(1 hunks)compiled_starters/clojure/deps.edn(1 hunks)compiled_starters/clojure/src/http_server/core.clj(1 hunks)compiled_starters/clojure/your_program.sh(1 hunks)dockerfiles/clojure-1.12.0.Dockerfile(1 hunks)solutions/clojure/01-at4/code/.codecrafters/compile.sh(1 hunks)solutions/clojure/01-at4/code/.codecrafters/run.sh(1 hunks)solutions/clojure/01-at4/code/.gitattributes(1 hunks)solutions/clojure/01-at4/code/.gitignore(1 hunks)solutions/clojure/01-at4/code/README.md(1 hunks)solutions/clojure/01-at4/code/build.clj(1 hunks)solutions/clojure/01-at4/code/codecrafters.yml(1 hunks)solutions/clojure/01-at4/code/deps.edn(1 hunks)solutions/clojure/01-at4/code/src/http_server/core.clj(1 hunks)solutions/clojure/01-at4/code/your_program.sh(1 hunks)solutions/clojure/01-at4/diff/src/http_server/core.clj.diff(1 hunks)solutions/clojure/01-at4/explanation.md(1 hunks)starter_templates/clojure/code/.codecrafters/compile.sh(1 hunks)starter_templates/clojure/code/.codecrafters/run.sh(1 hunks)starter_templates/clojure/code/.gitignore(1 hunks)starter_templates/clojure/code/build.clj(1 hunks)starter_templates/clojure/code/deps.edn(1 hunks)starter_templates/clojure/code/src/http_server/core.clj(1 hunks)starter_templates/clojure/config.yml(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
solutions/clojure/01-at4/explanation.md
12-12: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🪛 LanguageTool
solutions/clojure/01-at4/code/README.md
[misspelling] ~7-~7: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...he web. In this challenge, you'll build a HTTP/1.1 server that is capable of serv...
(EN_A_VS_AN)
compiled_starters/clojure/README.md
[misspelling] ~7-~7: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...he web. In this challenge, you'll build a HTTP/1.1 server that is capable of serv...
(EN_A_VS_AN)
🪛 Hadolint (2.12.0)
dockerfiles/clojure-1.12.0.Dockerfile
[error] 10-10: invalid flag: --exclude
(DL1000)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test_course_definition / test (clojure)
🔇 Additional comments (22)
solutions/clojure/01-at4/code/.gitattributes (1)
1-1: Correct.gitattributesto normalize line endings.Applying
* text=autoensures consistent EOL handling across platforms in this subtree.compiled_starters/clojure/deps.edn (2)
5-7: Dependencies pinned correctly.
You’ve specified Clojure1.12.0and tools.cli1.1.230. These look good. If you anticipate needing logging or other utility libs, now is a good time to add them.
10-13: Validate build alias configuration.
The:buildalias points tobuild/uber. Please confirm thatcompiled_starters/clojure/build.cljdefines abuildnamespace with anuberfunction matching this alias.solutions/clojure/01-at4/code/codecrafters.yml (1)
11-11: Ensure Clojure version consistency.
Thelanguage_pack: clojure-1.12.0aligns with yourdeps.ednversion—nice consistency.starter_templates/clojure/config.yml (1)
1-3: Template configuration looks correct.
required_executable: cljanduser_editable_file: src/http_server/core.cljmatch the expected Clojure starter layout.compiled_starters/clojure/codecrafters.yml (1)
11-11: Language pack alignment confirmed.
language_pack: clojure-1.12.0is consistent with your build configuration.starter_templates/clojure/code/.codecrafters/compile.sh (1)
1-12: Compilation script looks correct
The script properly exits on failure and invokes the:buildalias as expected. Ensurebuild.cljdefinesbuild/uberto match this alias.starter_templates/clojure/code/deps.edn (1)
1-16: Valid deps.edn configuration
The source paths, dependencies, and:buildalias are correctly defined for building the uberjar.solutions/clojure/01-at4/code/.codecrafters/compile.sh (1)
1-12: Compilation script looks correct
Matches the starter template’s compile step and correctly runs the build alias.solutions/clojure/01-at4/code/deps.edn (1)
1-16: Valid deps.edn configuration
Dependencies and the:buildalias are properly set up for the solution project.compiled_starters/clojure/.codecrafters/compile.sh (1)
1-11: Compile script is solid.It correctly invokes the Clojure build alias and exits on any failure.
starter_templates/clojure/code/.codecrafters/run.sh (1)
1-11: Run script invocation is correct.The script uses
execto forward signals and passes through arguments properly.dockerfiles/clojure-1.12.0.Dockerfile (1)
10-10: 🛠️ Refactor suggestion
⚠️ Potential issueInvalid Dockerfile COPY flags.
Using
COPY --excludetriggers syntax errors under standard Docker builds (Hadolint DL1000). Exclusions should be handled via a.dockerignorefile instead of inline flags.Proposed fix:
-COPY --exclude=.git --exclude=README.md . /app +COPY . /appAnd add a
.dockerignoreat the project root:.git README.md⛔ Skipped due to learnings
Learnt from: SughiY PR: codecrafters-io/build-your-own-http-server#112 File: dockerfiles/clojure-1.12.Dockerfile:10-10 Timestamp: 2025-02-17T19:29:27.993Z Learning: The `--exclude` flag in COPY commands is supported in Dockerfile syntax 1.7-labs and should be retained for optimizing layer caching, despite static analysis tools like Hadolint flagging it as an error.🧰 Tools
🪛 Hadolint (2.12.0)
[error] 10-10: invalid flag: --exclude
(DL1000)
solutions/clojure/01-at4/code/.gitignore (1)
1-15: LGTM! Comprehensive Clojure .gitignore patterns.The .gitignore file correctly covers all standard Clojure build artifacts, Leiningen files, and deps.edn cache directories.
compiled_starters/clojure/.gitignore (1)
1-15: LGTM! Consistent .gitignore patterns.The .gitignore file maintains consistency with the solutions directory and includes all necessary Clojure build artifacts.
starter_templates/clojure/code/.gitignore (1)
1-15: LGTM! Comprehensive Clojure .gitignoreThe .gitignore entries appropriately cover all common Clojure build artifacts, tooling files, and dependency caches. This follows standard practices for Clojure projects.
starter_templates/clojure/code/src/http_server/core.clj (1)
18-19: LGTM! Correct binding syntax for debug outputThe
(binding [*out* *err*] ...)syntax is correct and properly handles redirecting stdout to stderr for debugging. This is not the source of your syntax error issue.compiled_starters/clojure/README.md (1)
1-39: Well-structured documentationThe README provides clear instructions for the HTTP server challenge, including setup steps and submission process. The content is comprehensive and user-friendly.
🧰 Tools
🪛 LanguageTool
[misspelling] ~7-~7: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...he web. In this challenge, you'll build a HTTP/1.1 server that is capable of serv...(EN_A_VS_AN)
compiled_starters/clojure/your_program.sh (1)
1-24: LGTM: Well-structured shell script.The script follows best practices with proper error handling, directory context management, and clear separation between local and remote execution paths.
solutions/clojure/01-at4/code/build.clj (1)
1-22: LGTM: Well-structured build configuration.The build script properly uses clojure.tools.build.api with appropriate clean and uber functions. The configuration correctly specifies the main namespace and output paths.
starter_templates/clojure/code/build.clj (2)
1-3: LGTM: Standard Clojure build namespace setup.The namespace declaration and imports are correctly structured for a build script.
13-22:✅ Verification successful
Verify namespace compilation target exists.
The build script compiles
http-server.corenamespace. Ensure this namespace exists and is properly structured, as compilation errors here could contribute to the hanging test issues mentioned in the PR.
🏁 Script executed:
#!/bin/bash # Description: Verify the http-server.core namespace exists and check for syntax errors # Expected: Find the core.clj file and check for basic syntax issues # Search for the core namespace file fd -t f "core.clj" --exec grep -l "ns.*http-server.core" # Check for common Clojure syntax issues in core files fd -t f "core.clj" --exec clj -M -e "(require 'clojure.spec.alpha) (clojure.spec.alpha/valid? ::clojure.core.specs.alpha/ns-form (read-string (slurp \"{}\")))"Length of output: 587
🏁 Script executed:
#!/bin/bash # Verify the namespace declaration in the starter template core.clj # Check for the ns declaration of http-server.core grep -R "ns\s\+http-server\.core" starter_templates/clojure/code/src/http_server/core.clj # Show the first 10 lines of the file to confirm structure sed -n '1,10p' starter_templates/clojure/code/src/http_server/core.cljLength of output: 333
Namespace http-server.core Verified
The
http-server.corenamespace is defined instarter_templates/clojure/code/src/http_server/core.cljand follows the correct structure:(ns http-server.core (:require [clojure.java.io :as io]) (:import [java.net ServerSocket]) (:gen-class))No further changes are needed here.
Modified-by: Brandon Olivier <[email protected]>
|
I don't totally understand how As is, the code works when run from diff --git a/starter_templates/clojure/code/src/http_server/core.clj b/starter_templates/clojure/code/src/http_server/core.clj
index 3129ce6..7de61aa 100644
--- a/starter_templates/clojure/code/src/http_server/core.clj
+++ b/starter_templates/clojure/code/src/http_server/core.clj
@@ -25,6 +25,7 @@
;; Uncomment this block to pass the first stage
- ;;(serve)
+ ;; (println "uncommented")
+ (serve)
)@andy1li Am I missing context for how this needs to be run? |
|
Does this not work? 🤔 
|
|
It consistently fails like this: The code in The odd thing is if I uncomment it by hand in the starter template, the test passes. |
|
Just ran Perhaps you're running an older and customized version of Mind upgrading your |
|
That's possible. I'll double check. I've had a few weird issues that I think might also be related to Bun and/or Docker versions. Can you check which versions you're using slash point me to where the correct versions might be listed? |
|
Just checked, our recent changes to Oh, we did pin My local versions: 
|
|
Hmm, I'm a major version ahead on Docker, that could be the issue. The others are fine. |
|
@andy1li I grabbed that version of Docker and it's still failing for me. If it's working on the server/your machine, that's great news though 😅 |
|
Let me know when this PR is out of the WIP status. |
|
@andy1li A decision needs to be made on how to handle stderr/stdout in the starter template here. I think it's fine as is, but it could be confusing for people new to Clojure. I'm not sure there's an easy way around that for general case printing to stderr. I tracked down what causes some of that awkwardness and have a couple suggestions in an course-sdk issue |
|
I suspect it's failing for me because of timing issues. I suspect needing to download all the deps plus jvm startup time puts it past the timeout. |
|
@andy1li this all looks good to me. If you're happy with it, we can merge away. |
This is still a work-in-progress, but I wanted to see if I could get a couple clarifications on what's blocking me. The tests are failing, but I can't quite figure it out.
When I run
course-sdk test clojurethe code hangs. I figured out where the syntax error is coming from and added a quick fix for that here. I'm not sure why the code times out when I run the tests through here, but it behaves as I'd expect locally when I run it.Summary by CodeRabbit
New Features
Documentation
Chores