WIP: components-first-release #7
Conversation
adds: build.clj file and release.yml
WalkthroughThe pull request introduces new GitHub Actions workflows for automating the release processes of Node.js and Clojure projects. The Changes
Assessment against linked issues
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range comments (1)
src/mockingbird/components/profile.cljs (1)
Potential Issues with Removed Constants and Functions
The constants
gravatar-base-url,pfp-styles, and the functionget-simple-avatar-urlare still referenced in the codebase, which may lead to broken functionality if they are removed without updating these references.
gravatar-base-urlis used insrc/mockingbird/components/profile.cljs.pfp-stylesis used insrc/mockingbird/components/profile.cljsandsrc/mockingbird/components/image.cljs.get-simple-avatar-urlis used insrc/mockingbird/components/profile.cljs.Please verify and update these references to ensure that the functionality is not disrupted. The removal of
auth0-cdn-base-urldoes not appear to affect the codebase.Analysis chain
Line range hint
1-1: Verify the impact of the removed constants and functions.The removal of the constants
gravatar-base-url,auth0-cdn-base-url,pfp-styles, and the functionget-simple-avatar-urlsuggests a potential redesign of how user avatars are generated or retrieved.Ensure that these changes do not break any existing functionality and that the new approach to handling user avatars is properly implemented and tested.
Run the following script to verify the usage of the removed entities:
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the removed constants and functions. # Test 1: Search for the usage of `gravatar-base-url`. Expect: No occurrences. rg --type clojure -w $'gravatar-base-url' # Test 2: Search for the usage of `auth0-cdn-base-url`. Expect: No occurrences. rg --type clojure -w $'auth0-cdn-base-url' # Test 3: Search for the usage of `pfp-styles`. Expect: No occurrences. rg --type clojure -w $'pfp-styles' # Test 4: Search for the usage of `get-simple-avatar-url`. Expect: No occurrences. rg --type clojure -w $'get-simple-avatar-url'Length of output: 577
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (12)
- .github/workflows/release.yml (1 hunks)
- README.md (1 hunks)
- build.clj (1 hunks)
- deps.edn (2 hunks)
- package.json (1 hunks)
- src/mockingbird/components/aside.cljs (2 hunks)
- src/mockingbird/components/icon.cljs (1 hunks)
- src/mockingbird/components/image.cljs (2 hunks)
- src/mockingbird/components/popup.cljs (4 hunks)
- src/mockingbird/components/profile.cljs (1 hunks)
- src/mockingbird/helpers/props.cljs (1 hunks)
- test/unit/components/props_test.cljs (1 hunks)
Additional context used
LanguageTool
README.md
[style] ~32-~32: As a shorter alternative for ‘able to’, consider using “can”.
Context: ...ribute our software via Clojars, so you are able to just plug and play! ``` clj ;; TODO ...(BE_ABLE_TO)
[uncategorized] ~43-~43: Loose punctuation mark.
Context: ...ay!clj ;; TODO# Usage ;; TODO # Testing Locally And you are...(UNLIKELY_OPENING_PUNCTUATION)
[style] ~49-~49: Consider a shorter alternative to avoid wordiness.
Context: ...done, you can clone and build this repo in order to see some examples of good usage of our ...(IN_ORDER_TO_PREMIUM)
Markdownlint
README.md
42-42: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
47-47: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
53-53: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
40-40: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
41-41: Expected: 1; Actual: 3
Multiple consecutive blank lines(MD012, no-multiple-blanks)
46-46: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
30-30: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
42-42: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
47-47: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
Additional comments not posted (30)
.github/workflows/release.yml (1)
23-24: Verify the secrets are correctly configured.Ensure that the
CLOJARS_USERNAMEandCLOJARS_PASSWORDsecrets are correctly configured in the repository settings before running this workflow.build.clj (1)
1-30: LGTM! The build file is well-structured and follows best practices.The build file is approved as it is well-structured, follows best practices for Clojure projects, and uses appropriate constants, functions, and libraries for building and deploying the project.
src/mockingbird/helpers/props.cljs (1)
44-44: LGTM!The change improves code readability by visually separating the parameters without affecting the functionality.
deps.edn (10)
3-3: LGTM, but verify the updated dependency.Updating the
org.clojure/clojurescriptdependency to version1.11.4is a good practice to access new features, optimizations, and bug fixes.However, ensure that the updated dependency is thoroughly tested to confirm that it behaves as expected and does not introduce any breaking changes or compatibility issues.
19-19: LGTM!Setting the default namespace for the
:buildalias tobuildis consistent with the other aliases and improves the usability of the build alias.
20-20: LGTM!The blank line improves code readability by visually separating the aliases without affecting the functionality.
21-21: LGTM!The comment provides clear and concise instructions for developers to run the build using the
:devalias, which is useful information.
22-22: LGTM!The comment provides clear and concise instructions for developers to set up Clojars credentials using environment variables, which is useful information.
23-23: LGTM!The comment provides clear and concise instructions for developers to deploy the project to Clojars using the
:deploy-clojarsalias, which is useful information.
24-24: LGTM!Adding the
:deploy-clojarsalias with theslipset/deps-deploylibrary for deployment is a good practice for managing deployments. Deriving the version of theslipset/deps-deploylibrary from thebuild/versionensures consistency between the build and deployment versions.
25-26: LGTM!Specifying the
:exec-fnand:exec-argsfor the:deploy-clojarsalias provides a structured approach to executing the deployment process. The:exec-fnspecifies the function to execute for deployment, and the:exec-argsspecifies the arguments for the deployment function, including the installer type, release signing options, and the necessary files for deployment.
27-28: LGTM!Specifying the
:installerand:sign-releases?options for the:exec-argsof the:deploy-clojarsalias provides flexibility in configuring the deployment process. Setting the:installeroption to:remoteindicates that the deployment will be done remotely, and setting the:sign-releases?option tofalseindicates that the releases will not be signed.
29-29: LGTM!Specifying the
:pom-fileand:artifactoptions for the:exec-argsof the:deploy-clojarsalias provides the necessary information for the deployment process. The:pom-fileoption specifies the path to the POM file for the project, and the:artifactoption specifies the path to the JAR file for the project, which is the artifact to be deployed.test/unit/components/props_test.cljs (4)
39-39: LGTM!The blank line improves code readability by visually separating the test cases without affecting the functionality.
43-45: LGTM!Updating the assertion for the
get-propsfunction to include the:shadowproperty alongside:roundnessenhances the test coverage by ensuring that the function can handle combinations of properties appropriately. The expected output string has been modified to include the shadow class, indicating that the function now correctly handles multiple properties and returns a string that includes the shadow class when both:roundnessand:shadoware provided.
46-46: LGTM!The blank line improves code readability by visually separating the test cases without affecting the functionality.
48-50: LGTM!Adjusting the assertion to check for the behavior of the
get-propsfunction when both:sizeand:roundnessare set tonilenhances the test coverage by ensuring that the function can handle nil values appropriately. The expected output remains unchanged as"w-16 h-16 ", reinforcing the function's capability to handle nil values gracefully without altering the default output.src/mockingbird/components/image.cljs (3)
8-11: LGTM!The changes to the
pfp-stylesmap are approved. The addition of multiple styles enhances the flexibility and visual diversity of thepfpcomponent.
28-37: LGTM!The changes to the
pfpcomponent are approved. The updated logic for applying styles based on thethemekey ensures that the component reflects the selected style dynamically, consistent with the updates to thepfp-stylesmap.
39-59: LGTM!The addition of the
imgcomponent is approved. The component's structure and customizable properties enhance modularity and reusability, allowing for consistent image rendering across the application.src/mockingbird/components/popup.cljs (3)
9-12: LGTM!The addition of the
error-stylesmap is approved. The styles encapsulate CSS class mappings for error notifications, enhancing the visual representation and providing a consistent look for error messages.
14-16: LGTM!The addition of the
toast-data-stylesmap is approved. The categorization of styles based on notification types enhances clarity and maintainability, while the specific class attributes ensure appropriate visual differentiation between informational and error notifications.
64-64: LGTM!The changes to the
toastcomponent are approved. The utilization of thetoast-data-stylesmap enhances clarity and maintainability by providing a structured approach to styling notifications and ensuring appropriate categorization of visual elements.package.json (4)
26-26: LGTM!The changes to the
watchscript are approved. The consolidation of the watch functionality under a single script improves usability and clarity, while the removal of theapp:watchscript reduces redundancy and streamlines the script management.
27-27: SkippedThe
replscript is an existing script that has not been changed in this pull request. Commenting on it is unnecessary.
28-28: LGTM!The addition of the
releasescript is approved. It provides a structured approach to managing the release process and enhances the build and deployment workflow, making it easier for developers to prepare the application for production.
29-29: LGTM!The addition of the
shadow:releasescript is approved. It complements thereleasescript and provides a clear separation of concerns, making the release process more modular and maintainable.README.md (2)
27-28: LGTM!The change improves accessibility by providing a direct link to the Node.js download page.
47-49: LGTM!The change provides a clearer context for users and enhances the practical guidance.
Tools
LanguageTool
[style] ~49-~49: Consider a shorter alternative to avoid wordiness.
Context: ...done, you can clone and build this repo in order to see some examples of good usage of our ...(IN_ORDER_TO_PREMIUM)
Markdownlint
47-47: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
47-47: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
src/mockingbird/components/aside.cljs (1)
12-23: LGTM!The changes to the
mockfunction enhance the component's reusability and configurability, enabling it to adapt to different contexts based on the provided props.
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (14)
- .github/workflows/ci-tests.yml (1 hunks)
- .github/workflows/release.yml (1 hunks)
- .gitignore (1 hunks)
- README.md (1 hunks)
- build.clj (1 hunks)
- deps.edn (2 hunks)
- docs/Roadmap.md (1 hunks)
- package.json (1 hunks)
- shadow-cljs.edn (1 hunks)
- src/dev/core.cljs (1 hunks)
- src/mockingbird/core.cljs (1 hunks)
- src/mockingbird/lib.cljc (1 hunks)
- src/mockingbird/node.cljs (1 hunks)
- test/unit/components/props_test.cljs (1 hunks)
Additional context used
LanguageTool
README.md
[grammar] ~32-~32: It appears that only one verb is needed.
Context: ...ribute our software via Clojars, so you are can just plug and play! ``` clj ;; TODO ...(THIS_IS_HAVE)
[uncategorized] ~43-~43: Loose punctuation mark.
Context: ...ay!clj ;; TODO# Usage ;; TODO # Testing Locally And you are...(UNLIKELY_OPENING_PUNCTUATION)
[style] ~49-~49: Consider a shorter alternative to avoid wordiness.
Context: ...done, you can clone and build this repo in order to see some examples of good usage of our ...(IN_ORDER_TO_PREMIUM)
Markdownlint
README.md
42-42: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
47-47: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
53-53: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
40-40: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
41-41: Expected: 1; Actual: 3
Multiple consecutive blank lines(MD012, no-multiple-blanks)
46-46: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
30-30: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
42-42: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
47-47: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
Additional comments not posted (23)
src/dev/core.cljs (1)
Line range hint
6-11: LGTM!The code changes are approved.
.gitignore (2)
23-23: LGTM!Excluding the entire
/resources/public/css/directory from version control is a good practice. It ensures that all generated or compiled CSS files within that directory are not tracked by Git.
25-25: LGTM!Excluding the
/resources/node/directory from version control is a good practice. It ensures that Node.js-related files, which are likely dependencies or generated files, are not tracked by Git.src/mockingbird/node.cljs (1)
7-9: LGTM!The code changes are approved.
src/mockingbird/core.cljs (1)
5-5: LGTM, but verify the impact of removing thehelix.domimport.The removal of the
helix.domimport suggests that its functionalities are no longer necessary for the current implementation of the application. This change reflects a simplification in the module's dependencies, which could lead to a more maintainable codebase if the removed functionality is indeed unused.However, without further context on the usage of
helix.domin the rest of the codebase, it is unclear whether this change will affect the rendering of components or the overall functionality of the application.To verify the impact of removing the
helix.domimport, run the following script:.github/workflows/ci-tests.yml (1)
31-31: LGTM!Using a specific version of the
setup-clojureaction instead ofmasteris a good practice. It enhances the stability and predictability of the CI pipeline by locking the action to a known version, thereby reducing the risk of unexpected behavior that could arise from changes in themasterbranch.The workflow will now consistently use the specified version of the
setup-clojureaction, ensuring that the environment setup for Clojure remains consistent across different runs of the CI process.build.clj (3)
10-12: LGTM!The code changes are approved.
14-26: LGTM!The code changes are approved.
28-34: LGTM!The code changes are approved.
.github/workflows/release.yml (1)
1-30: LGTM! The workflow is well-structured and follows best practices.The workflow is approved as it is well-structured, follows best practices for GitHub Actions, and uses appropriate actions, environments, and secrets for managing releases.
src/mockingbird/lib.cljc (1)
Line range hint
5-29: LGTM! The macro changes enhance reusability.The addition of the
^:exportmetadata to thedefncmacro is a positive change that improves the reusability and modularity of the macro. By making it accessible from other namespaces, the macro can now be utilized in other parts of the codebase or even in external projects that depend on this library.The core functionality of the macro remains intact, ensuring that existing usage is not affected. The macro continues to provide a convenient way to define React components using a Clojure-friendly syntax, with support for docstrings, metadata, and customizable feature flags.
Overall, these changes align with best practices in Clojure development by promoting code reuse and modularity. Great work!
deps.edn (2)
3-3: LGTM! The ClojureScript dependency is updated.The addition of the
org.clojure/clojurescriptdependency with version1.11.60is a significant update to the ClojureScript environment for the project. This change may introduce new features and optimizations, impacting the compilation and execution of ClojureScript code.
20-28: LGTM! The deployment configuration is well-structured.The addition of the
:deploy-clojarssection in thedeps.ednfile significantly enhances the deployment capabilities of the project. The inclusion of theslipset/deps-deploydependency and the specified execution function and arguments streamline the deployment process, allowing for remote installations and better management of project artifacts.The deployment configuration is well-structured, specifying the necessary details such as the installer type, options for signing releases, and paths to the required POM and artifact files. This structured approach improves the workflow and facilitates efficient deployment of the Clojure-based application.
test/unit/components/props_test.cljs (2)
8-35: LGTM!The test changes improve readability without altering the logic. The test cases cover various scenarios and the expected outputs match the previous implementation.
45-56: LGTM!The test changes improve the coverage by ensuring that the
get-propsfunction correctly handles multiple properties and nil values. The expected outputs have been updated to reflect the changes.package.json (3)
26-26: LGTM!The change to the
"watch"script is approved as it simplifies the command structure and enhances usability.
28-28: LGTM!The addition of the
"release"script is approved as it provides a clear command for preparing the application for production and signifies a more organized approach to the release process.
29-29: LGTM!The addition of the
"shadow:release"script is approved as it separates the release functionality into a dedicated script, making it easier to manage and maintain.README.md (1)
32-32: Update the sentence to use only "can".The previous comment suggested using "can" instead of "able to". The current change partially implemented this suggestion but introduced a grammatical error by using both "are" and "can".
Update the sentence to:
-We distribute our software via Clojars, so you are can just plug and play! +We distribute our software via Clojars, so you can just plug and play!Tools
LanguageTool
[grammar] ~32-~32: It appears that only one verb is needed.
Context: ...ribute our software via Clojars, so you are can just plug and play! ``` clj ;; TODO ...(THIS_IS_HAVE)
shadow-cljs.edn (4)
3-30: LGTM!The
:appbuild configuration looks good and follows the best practices:
- It separates the development and release configurations.
- It enables strict compiler checks with
warnings-as-errors.- It generates a source map for better debugging in production.
- It infers externs to reduce the bundle size.
- It enables advanced optimizations for better performance in production.
31-35: LGTM!The
:testsbuild configuration looks good and follows the best practices:
- It specifies a separate test directory.
- It uses a namespace regexp to identify test files.
- It specifies separate devtools settings for running tests.
37-40: LGTM!The
:ci-testsbuild configuration looks good and follows the best practices:
- It specifies a separate output file for CI tests.
- It uses a namespace regexp to identify test files.
- It disables static function optimization for better test coverage.
41-66: LGTM!The newly added
:node-libbuild configuration looks good and follows the best practices:
- It specifies a separate output directory and file for the Node.js library.
- It specifies an exports function to control the exported API.
- It separates the development and release configurations.
- It enables strict compiler checks with
warnings-as-errors.- It generates a source map for better debugging in production.
- It infers externs to reduce the bundle size.
- It enables advanced optimizations for better performance in production.
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (7)
- .github/workflows/release.yml (1 hunks)
- deps.edn (1 hunks)
- shadow-cljs.edn (1 hunks)
- src/mockingbird/components/image.cljs (2 hunks)
- src/mockingbird/css/tailwind.css (0 hunks)
- src/mockingbird/node.cljs (1 hunks)
- test/unit/components/props_test.cljs (0 hunks)
Files not reviewed due to no reviewable changes (2)
- src/mockingbird/css/tailwind.css
- test/unit/components/props_test.cljs
Additional comments not posted (11)
src/mockingbird/node.cljs (2)
7-9: LGTM!The
hellofunction is correctly implemented. It prints and returns the string "hello" as expected.
11-16: The past review comment suggesting to rename thepfpexport toimagefor consistency with the import name is still valid. Please address this feedback..github/workflows/release.yml (1)
1-29: LGTM! The workflow is well-structured and follows best practices.The workflow is approved as it is well-structured, follows best practices for GitHub Actions, and uses appropriate actions, environments, and secrets for managing releases.
To improve the workflow, consider adding a brief description at the beginning of the file to explain the purpose and trigger conditions of the workflow. For example:
# Workflow for releasing the project to Clojars # Triggered on pushing tags starting with 'v' or manually using workflow_dispatch name: Releasedeps.edn (1)
3-3: Verify compatibility of the updated ClojureScript version.Updating the ClojureScript dependency to version
1.11.60is a good practice to leverage new features and improvements. However, please ensure that this version is compatible with the codebase and other dependencies to avoid any potential breaking changes or issues.src/mockingbird/components/image.cljs (2)
8-11: LGTM!The changes to
pfp-stylesenhance the flexibility of thepfpcomponent by allowing it to adapt its styling based on the provided theme. The styles are well-defined, cover different visual variations, and have clear and intuitive naming.
39-60: LGTM!The new
imgcomponent is a great addition to the codebase. It allows for more modular and reusable code, enabling developers to easily render images with consistent styling and behavior across the application. The component's properties are well-defined, cover various aspects of image rendering, and include sensible default values. The component's structure is clean and follows the established patterns in the codebase.shadow-cljs.edn (5)
3-8: LGTM!The reformatting of the
:appbuild target configuration improves readability without altering the functionality. The specified options are suitable for a browser build target.
15-20: LGTM!The reformatting of the
:devconfiguration under the:appbuild target enhances readability without modifying the functionality. The specified options are suitable for a development build.
21-30: LGTM!The reformatting of the
:releaseconfiguration under the:appbuild target improves readability without altering the functionality. The specified options are suitable for a production release build.
31-35: LGTM!The reformatting of the
:testsbuild target configuration enhances readability without modifying the functionality. The specified options are suitable for a browser-based testing build target.
40-53: Verify the usage of themockingbird.node/generate-exportsfunction.The addition of the
:node-libbuild target is a great enhancement to generate a Node.js library. The specified options are suitable for a Node.js library build. However, please ensure that themockingbird.node/generate-exportsfunction referenced in the:exports-fnoption is implemented correctly and generates the desired library exports.Run the following script to verify the usage of the
mockingbird.node/generate-exportsfunction:Verification successful
The
mockingbird.node/generate-exportsfunction is correctly implemented.The
generate-exportsfunction is defined in thesrc/mockingbird/node.cljsfile and returns a JavaScript object with the expected exports. This implementation aligns with the intended purpose of generating exports for a Node.js library. No issues were found with the function's implementation.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of the `mockingbird.node/generate-exports` function. # Test: Search for the function definition. # Expect: The function should be defined in the `mockingbird.node` namespace. rg --type clojure $'defn generate-exports'Length of output: 95
Script:
#!/bin/bash # Description: Extract the implementation of the `mockingbird.node/generate-exports` function for review. # Extract the function implementation from the source file. rg --type clojure -A 20 $'defn generate-exports' src/mockingbird/node.cljsLength of output: 196
| @@ -1,5 +1,7 @@ | |||
| {:paths ["src" "resources"] | |||
| :deps {org.clojure/clojure {:mvn/version "1.11.1"} | |||
| org.clojure/clojurescript {:mvn/version "1.11.60"} | |||
| com.google.javascript/closure-compiler-unshaded {:mvn/version "v20220803"} | |||
There was a problem hiding this comment.
Consider using a more recent version of the Closure Compiler.
Adding the Closure Compiler dependency is a great choice for optimizing the JavaScript output. However, the specified version v20220803 is quite dated. Consider using a more recent version to ensure you have access to the latest optimizations and bug fixes.
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (1)
- README.md (1 hunks)
Additional context used
LanguageTool
README.md
[grammar] ~32-~32: It appears that only one verb is needed.
Context: ...ribute our software via Clojars, so you are can just plug and play! ``` clj ;; TODO ...(THIS_IS_HAVE)
[uncategorized] ~43-~43: Loose punctuation mark.
Context: ...ay!clj ;; TODO# Usage ;; TODO # Testing Locally You can clo...(UNLIKELY_OPENING_PUNCTUATION)
Markdownlint
README.md
42-42: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
47-47: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
53-53: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
40-40: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
41-41: Expected: 1; Actual: 3
Multiple consecutive blank lines(MD012, no-multiple-blanks)
46-46: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
30-30: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
42-42: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
47-47: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
Additional comments not posted (4)
README.md (4)
27-27: LGTM!Providing a direct link to the Node.js download page improves user accessibility.
28-28: LGTM!Providing a direct link to the npm downloads page improves user accessibility.
49-49: The past review comment is still valid.Please address the following suggestions from the past review comment:
- Remove "And you are now done" as it doesn't add value.
- Replace "in order to" with "to" for conciseness.
- Add a comma after "repo" for clarity.
-And you are now done, you can clone and build this repo in order to see some examples of good usage of our dear Mockingbird. +You can clone and build this repo, to see some examples of good usage of our dear Mockingbird.
32-32: Address the grammar issue.The past review comment suggesting to replace "able to" with "can" is still valid.
Additionally, please fix the grammar issue flagged by the static analysis hint:
-We distribute our software via Clojars, so you are can just plug and play! +We distribute our software via Clojars, so you can just plug and play!Tools
LanguageTool
[grammar] ~32-~32: It appears that only one verb is needed.
Context: ...ribute our software via Clojars, so you are can just plug and play! ``` clj ;; TODO ...(THIS_IS_HAVE)
There was a problem hiding this comment.
Actionable comments posted: 11
Outside diff range comments (2)
src/mockingbird/examples/main.cljs (1)
Line range hint
13-13: Address the TODO commentThere's a TODO comment indicating the need to create an example to demonstrate the components' capabilities. If you'd like, I can assist in implementing this example or open a GitHub issue to track this task.
src/mockingbird/components/selectdropdown.cljs (1)
Line range hint
5-5: Reminder: Address the TODO comment regarding the 'styles' map.The
stylesmap is currently empty, and there's a TODO comment indicating that styles need to be added. Populating this map is essential for applying themes correctly to the component.Would you like assistance in generating the styles or creating a GitHub issue to track this task?
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
Files selected for processing (10)
- .gitignore (1 hunks)
- package.json (4 hunks)
- resources/index.src.html (0 hunks)
- shadow-cljs.edn (1 hunks)
- src/mockingbird/components/filedropdown.cljs (1 hunks)
- src/mockingbird/components/selectdropdown.cljs (1 hunks)
- src/mockingbird/components/status.cljs (1 hunks)
- src/mockingbird/css/tailwind.css (0 hunks)
- src/mockingbird/examples/main.cljs (1 hunks)
- webpack.config.js (1 hunks)
Files not reviewed due to no reviewable changes (2)
- resources/index.src.html
- src/mockingbird/css/tailwind.css
Additional context used
Biome
webpack.config.js
[error] 1-1: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
Additional comments not posted (8)
.gitignore (2)
23-23: Verify necessity of excluding/resources/public/css/directoryExcluding the entire
/resources/public/css/directory may lead to important CSS files not being tracked in version control. If this directory contains source CSS files (like custom stylesheets) that are not generated, they should remain under version control. Consider only excluding generated files or specific subdirectories (e.g.,/resources/public/css/generated/).
25-25: Excluding/dist/directory is appropriateAdding
/dist/to.gitignoreis a good practice as it prevents committing generated distribution files to the repository, keeping it clean from build artifacts.src/mockingbird/components/filedropdown.cljs (1)
8-8: Ensure all usages ofform-dropdownare updated tofile-dropdownRenaming the component from
form-dropdowntofile-dropdownmay require updating all references to this component throughout the codebase to prevent runtime errors.Run the following script to verify that all occurrences of
form-dropdownhave been updated:Verification successful
To gather more conclusive information, I'll run additional scripts to verify the component renaming.
Let's try one more time with more basic Unix tools to verify the component renaming.
Based on the thorough search of the codebase, I can now provide a final response regarding the component renaming.
Component renaming verified successfully
The renaming of 'form-dropdown' to 'file-dropdown' has been completed correctly. There are no remaining instances of 'form-dropdown' in the codebase, and the new name 'filedropdown' is used consistently in the relevant file (src/mockingbird/components/filedropdown.cljs).
- The component is defined as 'filedropdown' in src/mockingbird/components/filedropdown.cljs
- No other occurrences of 'form-dropdown' were found in any Clojure files
No further action is needed regarding this renaming. The changes have been implemented correctly throughout the codebase.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find any occurrences of 'form-dropdown' in the codebase. # Test: Search for 'form-dropdown' in the codebase. # Expect: No occurrences found. rg --type clojure -F 'form-dropdown'Length of output: 36
Script:
#!/bin/bash # Verify ripgrep functionality and search for both component names # Check if ripgrep is available which rg # Search for 'form-dropdown' without file type restriction rg 'form-dropdown' # Search for 'file-dropdown' without file type restriction rg 'file-dropdown' # Fallback: Use fd to find Clojure files and grep them fd -e cljs -e cljc -e clj | xargs grep -n 'form-dropdown' fd -e cljs -e cljc -e clj | xargs grep -n 'file-dropdown'Length of output: 237
Script:
#!/bin/bash # Search for dropdown-related terms in Clojure files using find and grep # Find Clojure files and search for "form-dropdown" find . -type f \( -name "*.cljs" -o -name "*.cljc" -o -name "*.clj" \) -exec grep -Hn "form-dropdown" {} + # Find Clojure files and search for "file-dropdown" find . -type f \( -name "*.cljs" -o -name "*.cljc" -o -name "*.clj" \) -exec grep -Hn "file-dropdown" {} + # Find Clojure files and search for "dropdown" (to catch any variations) find . -type f \( -name "*.cljs" -o -name "*.cljc" -o -name "*.clj" \) -exec grep -Hn "dropdown" {} +Length of output: 1150
src/mockingbird/examples/main.cljs (1)
17-17: Confirm the removal of thenav-barcomponentThe
nav-barcomponent has been commented out in theappfunction. Please verify if this change is intentional. If it's temporary, consider adding a comment explaining the reason for disabling the navigation bar.src/mockingbird/components/status.cljs (1)
3-3: Correctly importdefncas a macroChanging the import of
defncto use:refer-macrosis appropriate sincedefncis a macro. This ensures that it's correctly recognized and expanded during compilation.package.json (2)
8-9: Verify compatibility of loader versions with Webpack 5The specified version for
"css-loader"is"^7.1.2", which does not exist. The latest version is around"^6.8.1". Additionally, ensure that both"babel-loader"("^9.2.1") and"css-loader"are compatible with Webpack 5 to prevent build issues.Apply this diff to correct the
"css-loader"version:- "css-loader": "^7.1.2", + "css-loader": "^6.8.1",
64-64: Verify the necessity of "use-sync-external-store"
"react"version"^18.2.0"includes theuseSyncExternalStoreAPI by default. Including"use-sync-external-store": "1.2.0"as a separate dependency may be unnecessary and could cause version conflicts. Please confirm if this dependency is required.src/mockingbird/components/selectdropdown.cljs (1)
9-9: Ensure all references toselect-dropdownare updated toselectdropdown.The function has been renamed from
select-dropdowntoselectdropdown. Please verify that all calls and references toselect-dropdownin the codebase have been updated to prevent any runtime errors.Run the following script to find any remaining references to
select-dropdown:Verification successful
Renaming verification successful: 'select-dropdown' to 'selectdropdown'
The renaming from 'select-dropdown' to 'selectdropdown' has been successfully implemented across the codebase. No occurrences of the old name were found, and the new name is correctly used in the expected locations. No further action is required.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all references to `select-dropdown` in the codebase. # Test: Search for `select-dropdown` function calls. Expect: No occurrences. rg --type clojure -w 'select-dropdown'Length of output: 38
Script:
#!/bin/bash # Description: Verify renaming of select-dropdown to selectdropdown # Test 1: Verify ripgrep functionality echo "Test 1: Verifying ripgrep functionality" rg --type clojure "defnc" # Test 2: Search for old function name (case-sensitive) echo "Test 2: Searching for 'select-dropdown' (case-sensitive)" rg --type clojure -w 'select-dropdown' # Test 3: Search for new function name (case-sensitive) echo "Test 3: Searching for 'selectdropdown' (case-sensitive)" rg --type clojure -w 'selectdropdown' # Test 4: Case-insensitive search for variations echo "Test 4: Searching for variations (case-insensitive)" rg --type clojure -i 'select(-?)dropdown'Length of output: 4231
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range comments (1)
src/mockingbird/components/image.cljs (1)
Incomplete refactoring from 'pfp' to 'image' component detected
The renaming process from 'pfp' to 'image' is incomplete, which may lead to confusion and potential bugs. Here's what we found:
- Both 'pfp.cljs' and 'image.cljs' exist in the codebase.
- The 'image' component is missing some functionality (pfp-styles) present in the 'pfp' component.
- There are no usages of the new 'image' component in the codebase.
- The 'pfp' component is still being used in other files.
To complete the refactoring process, please:
- Update 'image.cljs' to include all necessary functionality from 'pfp.cljs', such as the 'pfp-styles' definition.
- Replace all usages of the 'pfp' component with the 'image' component throughout the codebase.
- Once all usages are updated, remove the 'pfp.cljs' file.
- Update any import statements referencing 'pfp' to use 'image' instead.
After completing these steps, run a thorough test to ensure no functionality is broken.
Analysis chain
Line range hint
9-29: Ensure all references topfpare updated toimagethroughout the codebase.Since the
pfpcomponent has been renamed toimage, please verify that all imports and usages have been updated to prevent any broken references.Run the following script to check for any remaining references to
pfp:
To gather more information about the renaming process and its current state, let's run the following scripts:
To gather the final pieces of information needed to provide an accurate assessment, let's run the following scripts:
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find any remaining references to 'pfp' in the codebase. rg --type clojure 'pfp'Length of output: 440
Script:
#!/bin/bash # Check for the existence of 'image.cljs' and display its contents if it exists echo "Checking for image.cljs:" fd -t f image.cljs echo "\nContents of image.cljs (if it exists):" cat $(fd -t f image.cljs) 2>/dev/null || echo "File not found" echo "\nSearching for usage of 'image' component:" rg --type clojure '\[mockingbird\.components\.image :refer \[image\]\]' rg --type clojure '\(\\$ image'Length of output: 1352
Script:
#!/bin/bash echo "Contents of pfp.cljs:" cat $(fd -t f pfp.cljs) 2>/dev/null || echo "File not found" echo "\nSearching for any usage of 'image' component (including partial matches):" rg --type clojure 'image' -C 2Length of output: 3884
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (8)
- .github/workflows/release.yml (1 hunks)
- package.json (4 hunks)
- shadow-cljs.edn (1 hunks)
- src/mockingbird/components/hero.cljs (1 hunks)
- src/mockingbird/components/image.cljs (2 hunks)
- src/mockingbird/components/pfp.cljs (1 hunks)
- src/mockingbird/components/profile.cljs (0 hunks)
- webpack.config.js (1 hunks)
Files not reviewed due to no reviewable changes (1)
- src/mockingbird/components/profile.cljs
Additional context used
Biome
webpack.config.js
[error] 1-1: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
Additional comments not posted (6)
webpack.config.js (1)
1-25: LGTM! The webpack configuration is well-structured and follows best practices.The configuration appropriately handles JavaScript and CSS files with the correct loaders and output settings.
Tools
Biome
[error] 1-1: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
.github/workflows/release.yml (1)
1-28: LGTM! The GitHub Actions workflow is properly configured.The workflow efficiently automates the release process with appropriate triggers and steps.
src/mockingbird/components/hero.cljs (1)
7-7: Verify that thepfpimport path is correctly updated.Ensure that the import statement for
pfpreflects its new location inmockingbird.components.pfp, and that the component functions as expected.Run the following script to confirm the import paths:
Verification successful
Import path for
pfpis correctly updatedThe verification process confirms that the import statement
[mockingbird.components.pfp :refer [pfp]]insrc/mockingbird/components/hero.cljsis correct and up-to-date. This is the only occurrence of this import in the codebase, ensuring consistency.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all import statements of 'pfp' to ensure the paths are correct. rg --type clojure '\[mockingbird\.components\.pfp'Length of output: 136
src/mockingbird/components/pfp.cljs (1)
1-37: LGTM! Thepfpcomponent is well-defined and follows best practices.The component is properly parameterized and utilizes styles effectively for flexibility and reusability.
package.json (1)
2-2: Consider the impact of renaming the package on existing users.Changing the package name from
"mockingbird"to"mockingbird-cljs"may affect users who depend on the original package. Ensure this change is intentional and communicate it appropriately to prevent any disruptions.shadow-cljs.edn (1)
41-57: LGTM: Addition of:npm-modulebuild targetThe new
:npm-modulebuild configuration is properly set up to facilitate the creation of an npm-compatible module, enhancing the project's integration with the npm ecosystem. The compiler options and build hooks are appropriately configured to ensure optimized and reliable module generation.
There was a problem hiding this comment.
Actionable comments posted: 5
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (5)
- .github/workflows/npm.yml (1 hunks)
- .github/workflows/release.yml (1 hunks)
- build.clj (1 hunks)
- deps.edn (2 hunks)
- package.json (4 hunks)
Additional comments not posted (4)
.github/workflows/npm.yml (1)
15-15: Verify the necessity of 'setup-clojure' action in this workflowSince this workflow is for publishing a Node.js package to npm, the inclusion of the
DeLaGuardo/[email protected]action may not be necessary unless Clojure is required during the build process. Please confirm if this action is needed..github/workflows/release.yml (1)
12-12: Verify the environment name 'env'The
environment: envsetting specifies the deployment environment. Please confirm thatenvis the correct environment name intended for this workflow. If not, consider updating it to the appropriate environment or removing it if unnecessary.package.json (2)
13-14: Verify the 'css-loader' versionThe specified version for
css-loaderis"^7.1.2", which may not yet be released or could be incorrect. As of now,css-loaderis at version6.x. Please confirm that this is the intended version.
2-4: Ensure consistency between 'main' entry point and build outputThe
mainfield is set to"dist/bundle.js", and thelibscript builds the package using Webpack. Please verify that the build output matches themainentry point and that thedist/bundle.jsfile is correctly generated during the build process.
fix: release yml actions versions and deps.edn versions
.github/workflows/npm.yml
Outdated
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - uses: "DeLaGuardo/[email protected]" |
There was a problem hiding this comment.
Ensure consistent usage of quotes in 'uses' statements
In your workflow, some uses statements include quotes while others do not. For consistency and clarity, it's recommended to either include or omit quotes uniformly across all uses statements.
Apply this diff to remove unnecessary quotes:
- uses: actions/checkout@v4
- - uses: "DeLaGuardo/[email protected]"
+ - uses: DeLaGuardo/[email protected]
- uses: actions/setup-node@v3
with:
node-version: "20"
- name: Build and package the library
run: |
npm ci &&
npm run lib
- - uses: "JS-DevTools/[email protected]"
+ - uses: JS-DevTools/[email protected]
with:
token: ${{ secrets.NPM_TOKEN }}Also applies to: 22-22
| @@ -15,4 +17,10 @@ | |||
| :clojure-lsp {:replace-deps {com.github.clojure-lsp/clojure-lsp-standalone {:mvn/version "2024.08.05_18.16.00-1"}} | |||
| :main-opts ["-m" "clojure-lsp.main"]} | |||
| :build {:deps {io.github.clojure/tools.build {:mvn/version "0.9.4"}} | |||
| :ns-default build}}} | |||
| :ns-default build} | |||
| :deploy-clojars {:extra-deps {slipset/deps-deploy {:mvn/version "RELEASE"}} | |||
There was a problem hiding this comment.
Specify an explicit version instead of "RELEASE" for 'slipset/deps-deploy'
Using {:mvn/version "RELEASE"} can lead to non-reproducible builds because it may resolve to different versions over time. Please specify an explicit version number to ensure build consistency and reliability.
Apply this diff to set a specific version (replace x.y.z with the desired version):
:deploy-clojars {:extra-deps {slipset/deps-deploy {:mvn/version "RELEASE"}}
+ {:mvn/version "x.y.z"}}
:exec-fn deps-deploy.deps-deploy/deploy
:exec-args {:installer :remote
:sign-releases? false
:pom-file "target/classes/META-INF/maven/com.github.moclojer/mockingbird/pom.xml"
:artifact "target/mockingbird-0.0.1.jar"}}}}Committable suggestion was skipped due to low confidence.
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (5)
- README.md (2 hunks)
- deps.edn (2 hunks)
- package.json (4 hunks)
- shadow-cljs.edn (1 hunks)
- src/mockingbird/components/button.cljs (2 hunks)
Additional context used
LanguageTool
README.md
[typographical] ~7-~7: Consider adding two commas here.
Context: ...ut ClojureScript (how you dare!) you can of course use our npm package into your JS applic...(OF_COURSE_COMMA)
[uncategorized] ~61-~61: Loose punctuation mark.
Context: ...ngen:clj ;; TODO# Usage ;; TODO # Testing Locally You can clo...(UNLIKELY_OPENING_PUNCTUATION)
Markdownlint
README.md
7-7: Expected: #installation; Actual: #Installation
Link fragments should be valid(MD051, link-fragments)
60-60: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
65-65: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
71-71: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
43-43: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
58-58: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
59-59: Expected: 1; Actual: 3
Multiple consecutive blank lines(MD012, no-multiple-blanks)
64-64: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
32-32: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
60-60: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
65-65: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
37-37: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
46-46: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
53-53: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
Additional comments not posted (24)
src/mockingbird/components/button.cljs (1)
Line range hint
11-26: LGTM!The changes to the
Buttoncomponent look good:
- The function name has been updated to
Button, which aligns with the naming convention for components.- The function parameters and logic remain unchanged, ensuring the component's functionality is preserved.
- The component's structure and rendering logic are correct.
deps.edn (6)
3-3: LGTM!Adding the ClojureScript dependency is a valid change, as it enables writing ClojureScript code in the project. The version
1.11.60is a stable release.
4-4: Consider using a more recent version of the Closure Compiler.Adding the Closure Compiler dependency is a great choice for optimizing the JavaScript output. However, the specified version
v20220803is quite dated. Consider using a more recent version to ensure you have access to the latest optimizations and bug fixes.
6-6: LGTM!Updating the version of the
lilactown/helixdependency to0.1.11is a valid change. Using the latest version ensures access to bug fixes, performance improvements, and new features.
21-21: Specify an explicit version instead of "RELEASE" for 'slipset/deps-deploy'Using
{:mvn/version "RELEASE"}can lead to non-reproducible builds because it may resolve to different versions over time. Please specify an explicit version number to ensure build consistency and reliability.
22-26: LGTM!Adding the
:deploy-clojarsalias with deployment configuration is a valid change, as it enables deploying the project to Clojars. The specified execution function and arguments are valid for theslipset/deps-deploylibrary, and the deployment configuration includes necessary options such as the installer, POM file, and artifact.
20-20: LGTM!Adding the
:buildalias with a dependency onio.github.clojure/tools.buildis a valid change, as it enables using the Clojure tools.build library for building the project. The version0.9.4is a stable release, and specifying a default namespacebuildis a common convention when using the tools.build library.package.json (13)
2-2: Ensure the name change is consistently applied.The change in the
"name"field from"mockingbird"to"mockingbird-lib"is approved. Please ensure that this name change is consistently applied throughout the project, including in documentation, configuration files, and any other relevant locations.
4-9: LGTM!The addition of the
"main"and"files"fields is approved. These fields provide clear entry points and documentation for users when the package is published and installed as a dependency.
12-14: LGTM!The update to the
"autoprefixer"dependency and the addition of"babel-loader"and"css-loader"to"devDependencies"are approved. These changes enhance the project's build capabilities and ensure up-to-date dependencies.
28-31: LGTM!The addition of the
"resolutions"field specifying versions for"react"and"react-dom"is approved. This ensures consistency and compatibility across the project and its dependencies, preventing version conflicts and ensuring a stable development environment.
37-37: LGTM!The modification of the
"watch"script to run"npx shadow-cljs watch app"is approved. This change simplifies the command structure and improves the development experience by providing a clearer command for watching the application.
38-38: LGTM!The addition of the
"repl"script running"npx shadow-cljs clj-repl"is approved. This script enhances the development workflow by providing a convenient way to start a Clojure REPL for interactive testing and debugging.
39-40: LGTM!The addition of the
"release"and"shadow:release"scripts is approved. These scripts streamline the release process by providing a higher-level script that orchestrates the release and runs the Shadow CLJS release command for theappbuild. This makes the release process more efficient and user-friendly for developers.
41-41: LGTM!The addition of the
"lib"script is approved. This script automates the process of building the library for distribution by running the Shadow CLJS release command for thenpm-modulebuild and then running Webpack with a specific configuration file. This makes it easier to prepare the code for distribution.
49-49: Move"@isaacs/cliui"to"devDependencies".As mentioned in the past review comment, the
"@isaacs/cliui"dependency should be moved to"devDependencies"if it is only used during the build process and not at runtime. This will result in a cleaner production build.
60-60: Move"glob"to"devDependencies".As mentioned in the past review comment, the
"glob"dependency should be moved to"devDependencies"if it is only used during the build process and not at runtime. This will result in a cleaner production build.
69-69: Move"rimraf"to"devDependencies".As mentioned in the past review comment, the
"rimraf"dependency should be moved to"devDependencies"if it is only used during the build process and not at runtime. This will result in a cleaner production build.
73-73: LGTM!The addition of the
"use-sync-external-store"dependency with a specific version"1.2.0"is approved. This dependency allows React components to subscribe to an external store, and specifying the exact version ensures consistency and reproducibility in the project.
74-75: Move"webpack"and"webpack-cli"to"devDependencies".As mentioned in the past review comment, the
"webpack"and"webpack-cli"dependencies should be moved to"devDependencies"since they are used during the build process and not at runtime. This will result in a cleaner production build.README.md (3)
29-30: LGTM!Adding direct links to the Node.js and npm installation pages enhances accessibility for users and helps them set up their environment easily.
65-75: Address the formatting issues mentioned in the previous review comment.The past review comment on the same line range is still applicable. Please make the following changes:
- Remove trailing spaces on lines 65 and 71.
- Remove multiple consecutive blank lines between lines 64-65.
Tools
Markdownlint
65-65: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
71-71: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
65-65: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
77-79: LGTM!Referencing the official guide for configuring shadow-cljs is helpful for users to gain a better understanding.
shadow-cljs.edn (1)
41-58: Addition of:npm-modulebuild target is well-configuredThe new
:npm-modulebuild target is correctly configured to generate an npm-compatible module. The configuration includes appropriate settings for output directory, asset path, exports, JavaScript options, module type, compiler options, build options, and build hooks. This enhancement will facilitate the integration of the project with the npm ecosystem and support the distribution of the components.
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range comments (1)
src/mockingbird/components/button.cljs (1)
Line range hint
11-26: Approve the changes to the Button component with a minor suggestion.The changes to the Button component look good:
- Renaming from
buttontoButtonfollows the common convention for React components.- Adding
^:exportmetadata allows the component to be used outside the ClojureScript module.- The component's functionality is preserved.
Consider adding a brief docstring to the
Buttoncomponent to explain its purpose and usage. This will improve the component's self-documentation. For example:(defnc ^:export Button "A customizable button component with various themes and types." [{:keys [class theme type disabled label on-click children] :or {type :default theme :mockingbird on-click (fn [_]) disabled false children "Insert some text"}}] ...)
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (1)
- src/mockingbird/components/button.cljs (2 hunks)
Additional comments not posted (1)
src/mockingbird/components/button.cljs (1)
29-30: Suggest a minor improvement for consistency.For consistency with the
Buttoncomponent, consider using destructuring in the function parameters:(defn ^:export button [{keys :keys}] (Button {:keys keys}))This change makes the function signature more idiomatic and consistent with the style used in the
Buttoncomponent.
There was a problem hiding this comment.
Actionable comments posted: 4
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
Files selected for processing (2)
- package.json (4 hunks)
- webpack.config.js (1 hunks)
Additional context used
Biome
webpack.config.js
[error] 1-1: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
[error] 25-27: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
Additional comments not posted (9)
webpack.config.js (2)
4-9: Good configuration for mode, context, and entry pointThe configuration for mode, context, and entry point is well-structured:
- Dynamic mode setting based on the
isProductionenvironment variable is a good practice.- The context is appropriately set to the 'target' directory.
- The entry point './mockingbird.lib.js' is clearly defined.
These settings provide a solid foundation for both development and production builds.
1-30: Well-structured Webpack configurationOverall, this Webpack configuration is well-structured and follows good practices:
- It provides different settings for development and production environments.
- The configuration covers all necessary aspects: entry point, module rules, and output.
- It allows for efficient processing of both JavaScript and CSS files.
The setup provides a solid foundation for building the project. Great job on creating a comprehensive and flexible Webpack configuration!
Tools
Biome
[error] 1-1: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
[error] 25-27: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
package.json (7)
5-9: LGTM: Appropriate files specified for package distribution.The addition of the "files" array is a good practice for controlling what gets published to npm. The inclusion of "dist/", "README.md", and "docs/" is appropriate for a library distribution.
12-14: LGTM: DevDependencies updated appropriately.The addition of babel-loader and css-loader as devDependencies is appropriate for the project's build process. Updating autoprefixer to the latest version is also good practice for maintaining up-to-date tooling.
77-77: LGTM: use-sync-external-store dependency added.The addition of
use-sync-external-storewith a specific version is good for managing external store subscriptions in React applications.Consider documenting its usage in the project to help other developers understand its purpose and implementation.
28-31: LGTM: Resolutions added for React dependencies.The addition of the "resolutions" field with specific versions for react and react-dom is a good practice to avoid dependency conflicts. The specified versions (^18.2.0) are recent and stable.
Ensure that these versions match the main dependencies:
#!/bin/bash # Check if react versions in dependencies match resolutions rg '"react": "\^18\.2\.0"' package.json rg '"react-dom": "\^18\.2\.0"' package.json
37-45: New build scripts added, but verify "watch" script functionality.The addition of new scripts for release, shadow-cljs operations, and webpack builds provides more granular control over the build and release process, which is good. However, the "watch" script has been simplified and might have removed some functionality.
Please verify that the simplified "watch" script still covers all necessary watch operations:
#!/bin/bash # Check for any other watch scripts that might have been removed rg '(watch|dev).*postcss' package.jsonConsider adding a composite "watch" script that runs both ClojureScript and CSS watching if needed:
- "watch": "npx shadow-cljs watch app", + "watch": "run-p shadow:watch postcss:watch", + "shadow:watch": "npx shadow-cljs watch app",
2-2: LGTM: Package name updated to reflect library status.The change from "mockingbird" to "mockingbird-lib" aligns with the PR objectives and suggests that this package is now intended to be used as a library. This is a good change that clarifies the package's purpose.
Ensure that this name change is reflected in other parts of the project:
Verification successful
LGTM: Package name change to "mockingbird-lib" is correctly implemented and references in code and documentation remain consistent. No further action required.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for references to the old package name rg --type-not json 'mockingbird(?!-lib)' -C 3Length of output: 281
Script:
#!/bin/bash # Search for references to the old package name with PCRE2 enabled rg --pcre2 --type-not json 'mockingbird(?!-lib)' -C 3Length of output: 23926
4-4: LGTM: Main file updated to use bundled distribution.The change to use "dist/bundle.js" as the main file aligns with the addition of webpack to the project. This suggests a move towards a bundled distribution, which is good for library distribution.
Ensure that the build process correctly generates this bundle:
| const path = require('path'); | ||
|
|
||
| module.exports = (env) => { |
There was a problem hiding this comment.
Use 'node:' protocol for built-in modules and good use of function for configuration
Great job on using a function for the Webpack configuration, allowing for dynamic settings based on the environment. However, let's improve the module import:
Apply this diff to update the import:
-const path = require('path');
+const path = require('node:path');This change aligns with Node.js best practices and explicitly indicates that 'path' is a core Node.js module.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const path = require('path'); | |
| module.exports = (env) => { | |
| const path = require('node:path'); | |
| module.exports = (env) => { |
Tools
Biome
[error] 1-1: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
There was a problem hiding this comment.
@Felipe-gsilva apparently this suggestion makes sense
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Outside diff range comments (1)
src/mockingbird/dev/core.cljs (1)
Line range hint
5-11: LGTM! Consider enhancing error logging.The
initfunction is correctly implemented and exported. It properly callsm/initand handles potential errors during initialization.Consider enhancing the error logging to include the stack trace for more detailed debugging information:
(defn ^:export init [] (try (m/init) (catch js/Error e - (js/console.error "Initialization failed:" (.message e))))) + (js/console.error "Initialization failed:" e))))This change will log the entire error object, including the stack trace, which can be helpful for debugging.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (4)
- deps.edn (1 hunks)
- shadow-cljs.edn (1 hunks)
- src/mockingbird/dev/core.cljs (1 hunks)
- src/mockingbird/dev/shadow/hooks.clj (1 hunks)
🔇 Additional comments (7)
src/mockingbird/dev/core.cljs (2)
Line range hint
1-11: Overall, the changes look good with minor suggestions.The namespace update to
mockingbird.dev.coreis appropriate and aligns with the project structure. Theinitfunction remains unchanged, maintaining its functionality. Consider the suggested enhancement for error logging to improve debugging capabilities.
1-3: LGTM! Namespace updated correctly.The namespace has been appropriately updated to
mockingbird.dev.core, which aligns with the project name. The required dependencymockingbird.coreis correctly aliased asm.Let's verify that there are no remaining references to the old namespace:
✅ Verification successful
Namespace change verified successfully. All references to the old namespace
dev.corehave been appropriately updated tomockingbird.dev.core.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining references to the old namespace 'dev.core' # Test: Search for 'dev.core' in all files. Expect: No results. rg --type-add 'clj:*.{clj,cljs,cljc}' --type clj 'dev\.core' # Test: Search for 'mockingbird.dev.core' in all files. Expect: At least one result (this file). rg --type-add 'clj:*.{clj,cljs,cljc}' --type clj 'mockingbird\.dev\.core'Length of output: 248
deps.edn (4)
3-3: LGTM: Addition of ClojureScript dependency.The addition of ClojureScript as a dependency is appropriate for the project's objectives. The specified version (1.11.60) is relatively recent, which is good for ensuring access to the latest features and bug fixes.
5-5: Consider updating to a more recent version of the Closure Compiler.While adding the Closure Compiler is beneficial for optimizing JavaScript output, the specified version
v20220803is quite outdated (from August 2022). As previously suggested, it would be advisable to use a more recent version to ensure access to the latest optimizations and bug fixes.
7-7: LGTM: Helix dependency version update.Updating the Helix dependency from version 0.1.9 to 0.1.11 is a good practice. This minor version bump likely includes bug fixes or small improvements that could benefit the project.
21-26: 🛠️ Refactor suggestionImprove deploy-clojars configuration for better reliability and flexibility.
As previously suggested, specify an explicit version for
slipset/deps-deployinstead of using "RELEASE". This ensures build reproducibility.Consider making the
pom-fileandartifactpaths more flexible::pom-file "target/classes/META-INF/maven/com.github.moclojer/mockingbird/pom.xml" :artifact "target/mockingbird-0.0.1.jar"These hardcoded values might cause issues with different versions or in CI/CD pipelines. Consider using environment variables or deriving these values dynamically.
Here's a suggested improvement:
:deploy-clojars {:extra-deps {slipset/deps-deploy {:mvn/version "0.2.0"}} ; Replace with the latest stable version :exec-fn deps-deploy.deps-deploy/deploy :exec-args {:installer :remote :sign-releases? false :pom-file ~(str "target/classes/META-INF/maven/com.github.moclojer/mockingbird/pom.xml") :artifact ~(str "target/mockingbird-" (System/getenv "PROJECT_VERSION") ".jar")}}This suggestion uses Clojure's reader macros to evaluate the strings at read-time, allowing for more dynamic configuration. Ensure to set the
PROJECT_VERSIONenvironment variable in your build process.shadow-cljs.edn (1)
41-58::npm-module` build configuration looks goodThe addition of the
:npm-modulebuild target is well-structured and correctly set up to produce an npm-compatible module. The specified exports, compiler options, and build hooks are appropriate for packaging your components for consumption via npm.
| :module-type :es6 | ||
| :compiler-options {:optimizations :advanced | ||
| :infer-externs :auto | ||
| :source-map true |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Evaluate the inclusion of source maps in the distributed npm module
Including source maps by setting :source-map true in your :compiler-options will generate source map files alongside your compiled code. While helpful for debugging, they can expose your original source code to end-users. If this is a concern for your project, especially in production builds, consider disabling source map generation.
Apply this diff to disable source map generation:
- :source-map true
+ :source-map false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| :source-map true | |
| :source-map false |
| :compiler-options {:static-fns false}} | ||
| :npm-module {:target :npm-module | ||
| :output-dir "./target/" | ||
| :asset-path "./target" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider removing :asset-path from the :npm-module build configuration
The :asset-path option is primarily used in browser builds to specify the base URL for assets at runtime. In an :npm-module build, this setting might be unnecessary since the module is consumed differently. Removing :asset-path can simplify the configuration unless it serves a specific purpose.
Apply this diff to remove :asset-path:
- :asset-path "./target"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| :asset-path "./target" |
| :infer-externs :auto | ||
| :source-map true | ||
| :warnings-as-errors true} | ||
| :build-options {:manifest-name "manifest.json"} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Review the necessity of :manifest-name in the :npm-module build
The :build-options {:manifest-name "manifest.json"} setting is typically used in browser environments to specify the name of the asset manifest file. In the context of an :npm-module build, this option may not be required and could be omitted to avoid generating unnecessary files.
Apply this diff to remove :manifest-name:
- :build-options {:manifest-name "manifest.json"}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| :build-options {:manifest-name "manifest.json"} |
| (defn copy-from-jar [jar-path file dest-dir file-name] | ||
| (try | ||
| (with-open [jar (java.util.jar.JarFile. jar-path)] | ||
| (if-let [entry (.getEntry jar file)] | ||
| (let [in (.getInputStream jar entry) | ||
| out (io/output-stream (io/file dest-dir file-name))] | ||
| (io/copy in out) | ||
| (prn "CSS file copied.")) | ||
| (throw (Exception. (str "File " file " not found in JAR."))))) | ||
| (catch Exception e | ||
| (println "Error copying file from JAR:" (.getMessage e))))) | ||
|
|
There was a problem hiding this comment.
Handle exceptions more specifically
Catching generic Exception can make debugging more difficult and may mask other errors. Consider catching more specific exceptions, such as FileNotFoundException or IOException, to provide more precise error handling.
For example, update the catch clauses as follows:
(defn copy-from-jar [jar-path file dest-dir file-name]
(try
;; existing code
(catch Exception e
- (log/error "Error copying file from JAR:" (.getMessage e)))))
+ (log/error e "Error copying file from JAR."))))Similarly, adjust other catch clauses in the code to catch specific exceptions where appropriate.
Also applies to: 34-41, 63-71
🛠️ Refactor suggestion
Use consistent logging instead of prn and println
The functions copy-from-jar are using prn and println for error messages and information logs. Consider using a logging library to handle log messages consistently throughout the application. This allows for better control over logging levels and outputs.
Apply this diff to replace prn and println with logging:
(defn copy-from-jar [jar-path file dest-dir file-name]
(try
(with-open [jar (java.util.jar.JarFile. jar-path)]
(if-let [entry (.getEntry jar file)]
(let [in (.getInputStream jar entry)
out (io/output-stream (io/file dest-dir file-name))]
(io/copy in out)
- (prn "CSS file copied."))
- (throw (Exception. (str "File " file " not found in JAR.")))))
+ (log/info "CSS file copied."))
+ (do
+ (log/error (str "File " file " not found in JAR."))
+ (throw (Exception. (str "File " file " not found in JAR."))))))
(catch Exception e
- (println "Error copying file from JAR:" (.getMessage e)))))
+ (log/error "Error copying file from JAR:" (.getMessage e)))))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| (defn copy-from-jar [jar-path file dest-dir file-name] | |
| (try | |
| (with-open [jar (java.util.jar.JarFile. jar-path)] | |
| (if-let [entry (.getEntry jar file)] | |
| (let [in (.getInputStream jar entry) | |
| out (io/output-stream (io/file dest-dir file-name))] | |
| (io/copy in out) | |
| (prn "CSS file copied.")) | |
| (throw (Exception. (str "File " file " not found in JAR."))))) | |
| (catch Exception e | |
| (println "Error copying file from JAR:" (.getMessage e))))) | |
| (defn copy-from-jar [jar-path file dest-dir file-name] | |
| (try | |
| (with-open [jar (java.util.jar.JarFile. jar-path)] | |
| (if-let [entry (.getEntry jar file)] | |
| (let [in (.getInputStream jar entry) | |
| out (io/output-stream (io/file dest-dir file-name))] | |
| (io/copy in out) | |
| (log/info "CSS file copied.")) | |
| (do | |
| (log/error (str "File " file " not found in JAR.")) | |
| (throw (Exception. (str "File " file " not found in JAR.")))))) | |
| (catch Exception e | |
| (log/error "Error copying file from JAR:" (.getMessage e))))) |
| [shadow.build :as build] | ||
| [shadow.cljs.util :as s.util])) | ||
|
|
||
| (def file (io/file "resources/public/assets/css/target.css")) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Rename the variable file to be more descriptive
The variable file is too generic and may cause confusion. Consider renaming it to something more descriptive like default-css-file to improve code readability.
Apply this diff to rename the variable:
-(def file (io/file "resources/public/assets/css/target.css"))
+(def default-css-file (io/file "resources/public/assets/css/target.css"))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| (def file (io/file "resources/public/assets/css/target.css")) | |
| (def default-css-file (io/file "resources/public/assets/css/target.css")) |
| (defn list-jar-files [jar-path] | ||
| (try | ||
| (with-open [jar (java.util.jar.JarFile. jar-path)] | ||
| (seq (filter #(.endsWith % "target.css") | ||
| (map #(.getName %) (enumeration-seq (.entries jar)))))) | ||
| (catch Exception e | ||
| (println "Error listing JAR contents:" (.getMessage e))))) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace println with a logging framework
In the list-jar-files function, using println for error messages is not ideal for production code. Implement a logging framework to manage log messages consistently.
Apply this diff to use logging:
(defn list-jar-files [jar-path]
(try
(with-open [jar (java.util.jar.JarFile. jar-path)]
(seq (filter #(.endsWith % "target.css")
(map #(.getName %) (enumeration-seq (.entries jar))))))
(catch Exception e
- (println "Error listing JAR contents:" (.getMessage e)))))
+ (log/error "Error listing JAR contents:" (.getMessage e)))))Committable suggestion was skipped due to low confidence.
| (defn retrieve-css | ||
| ([] | ||
| (retrieve-css nil)) | ||
| ([css-path] | ||
| (let [css-file (if (nil? css-path) | ||
| file | ||
| (io/file css-path))] | ||
| css-file))) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify the retrieve-css function
The retrieve-css function can be simplified by eliminating unnecessary variable bindings and using the or function to provide default values. This makes the code more concise and idiomatic.
Apply this diff to simplify the function:
(defn retrieve-css
- ([]
- (retrieve-css nil))
- ([css-path]
- (let [css-file (if (nil? css-path)
- file
- (io/file css-path))]
- css-file)))
+ ([]
+ default-css-file)
+ ([css-path]
+ (io/file (or css-path default-css-file))))Committable suggestion was skipped due to low confidence.
| (defn get-target-css ^:export | ||
| {:shadow.build/stage :configure} | ||
| [{::build/keys [mode] :as build-state} | ||
| {:keys [path file-name] | ||
| :or {path "resources/public/assets/css/target.css" | ||
| file-name "target.css"}}] | ||
| (let [file-name (if (str/includes? path ".css") | ||
| (last (str/split path #"/")) | ||
| file-name) | ||
| path (if (str/includes? path file-name) | ||
| (str/replace path file-name "") | ||
| path) | ||
| css-file (retrieve-css (str path file-name)) | ||
| jar-path (some #(when (re-find #"mockingbird" %) %) | ||
| (get-in (deps/create-basis | ||
| (deps/find-edn-maps | ||
| (io/file "deps.edn"))) | ||
| [:classpath-roots])) | ||
| jar-files (list-jar-files jar-path) | ||
| css-jar-file (first jar-files)] | ||
| (prn jar-path) | ||
| (if (nil? css-jar-file) | ||
| (prn "CSS file not found in the JAR.") | ||
| (if (.exists css-file) | ||
| (prn "CSS file already exists. No action needed.") | ||
| (try | ||
| (prn "CSS file does not exist. Copying from .jar...") | ||
| (copy-from-jar jar-path css-jar-file path file-name) | ||
| (catch Exception e | ||
| (prn "Error copying CSS file:" (.getMessage e))))))) | ||
| build-state) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor nested if statements for better readability
The get-target-css function contains nested if statements that make the code harder to read and maintain. Refactoring using cond or restructuring the logic can improve clarity.
Consider refactoring the code as follows:
(defn get-target-css
{:shadow.build/stage :configure}
[{::build/keys [mode] :as build-state}
{:keys [path file-name]
:or {path "resources/public/assets/css/target.css"
file-name "target.css"}}]
(let [file-name (if (str/includes? path ".css")
(last (str/split path #"/"))
file-name)
path (if (str/includes? path file-name)
(str/replace path file-name "")
path)
css-file (retrieve-css (str path file-name))
jar-path (some #(when (re-find #"mockingbird" %) %)
(get-in (deps/create-basis
(deps/find-edn-maps
(io/file "deps.edn")))
[:classpath-roots]))
jar-files (list-jar-files jar-path)
css-jar-file (first jar-files)]
- (prn jar-path)
- (if (nil? css-jar-file)
- (prn "CSS file not found in the JAR.")
- (if (.exists css-file)
- (prn "CSS file already exists. No action needed.")
- (try
- (prn "CSS file does not exist. Copying from .jar...")
- (copy-from-jar jar-path css-jar-file path file-name)
- (catch Exception e
- (prn "Error copying CSS file:" (.getMessage e)))))))
+ (log/debug "JAR path:" jar-path)
+ (cond
+ (nil? css-jar-file)
+ (log/warn "CSS file not found in the JAR.")
+
+ (.exists css-file)
+ (log/info "CSS file already exists. No action needed.")
+
+ :else
+ (try
+ (log/info "CSS file does not exist. Copying from JAR...")
+ (copy-from-jar jar-path css-jar-file path file-name)
+ (catch Exception e
+ (log/error "Error copying CSS file:" (.getMessage e))))))
build-state)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| (defn get-target-css ^:export | |
| {:shadow.build/stage :configure} | |
| [{::build/keys [mode] :as build-state} | |
| {:keys [path file-name] | |
| :or {path "resources/public/assets/css/target.css" | |
| file-name "target.css"}}] | |
| (let [file-name (if (str/includes? path ".css") | |
| (last (str/split path #"/")) | |
| file-name) | |
| path (if (str/includes? path file-name) | |
| (str/replace path file-name "") | |
| path) | |
| css-file (retrieve-css (str path file-name)) | |
| jar-path (some #(when (re-find #"mockingbird" %) %) | |
| (get-in (deps/create-basis | |
| (deps/find-edn-maps | |
| (io/file "deps.edn"))) | |
| [:classpath-roots])) | |
| jar-files (list-jar-files jar-path) | |
| css-jar-file (first jar-files)] | |
| (prn jar-path) | |
| (if (nil? css-jar-file) | |
| (prn "CSS file not found in the JAR.") | |
| (if (.exists css-file) | |
| (prn "CSS file already exists. No action needed.") | |
| (try | |
| (prn "CSS file does not exist. Copying from .jar...") | |
| (copy-from-jar jar-path css-jar-file path file-name) | |
| (catch Exception e | |
| (prn "Error copying CSS file:" (.getMessage e))))))) | |
| build-state) | |
| (defn get-target-css ^:export | |
| {:shadow.build/stage :configure} | |
| [{::build/keys [mode] :as build-state} | |
| {:keys [path file-name] | |
| :or {path "resources/public/assets/css/target.css" | |
| file-name "target.css"}}] | |
| (let [file-name (if (str/includes? path ".css") | |
| (last (str/split path #"/")) | |
| file-name) | |
| path (if (str/includes? path file-name) | |
| (str/replace path file-name "") | |
| path) | |
| css-file (retrieve-css (str path file-name)) | |
| jar-path (some #(when (re-find #"mockingbird" %) %) | |
| (get-in (deps/create-basis | |
| (deps/find-edn-maps | |
| (io/file "deps.edn"))) | |
| [:classpath-roots])) | |
| jar-files (list-jar-files jar-path) | |
| css-jar-file (first jar-files)] | |
| (log/debug "JAR path:" jar-path) | |
| (cond | |
| (nil? css-jar-file) | |
| (log/warn "CSS file not found in the JAR.") | |
| (.exists css-file) | |
| (log/info "CSS file already exists. No action needed.") | |
| :else | |
| (try | |
| (log/info "CSS file does not exist. Copying from JAR...") | |
| (copy-from-jar jar-path css-jar-file path file-name) | |
| (catch Exception e | |
| (log/error "Error copying CSS file:" (.getMessage e)))))) | |
| build-state) |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (1)
- tailwind.config.js (2 hunks)
🔇 Additional comments (3)
tailwind.config.js (3)
Line range hint
1-74: Overall, the changes improve configuration maintainability and align with modern Tailwind CSS practices.The updates to the Tailwind configuration file are well-aligned with the PR objectives of supporting the transition of components and refining the project structure. The changes to the
contentproperty, color definitions, and overall simplification of the configuration are positive improvements.However, it's crucial to verify that these changes don't introduce any breaking changes to existing styles. Please run the suggested verification scripts and update any affected styles if necessary.
49-49: Streamlined gray color palette. Verify no breaking changes.The removal of the
50shade from thegraycolor definition simplifies the configuration. However, ensure that this removal doesn't break any existing styles.Please run the following script to check for any usage of the removed gray-50 color:
#!/bin/bash # Description: Check for usage of removed gray-50 color # Test: Search for gray-50 usage in frontend files rg --type-add 'frontend:*.{cljs,cljc,clj,edn}' --type frontend 'gray-50' # Test: Search for gray-50 usage in HTML files rg --type html 'gray-50' public/If the script returns any results, consider keeping the
50shade or updating the affected styles.
45-46: Simplified color definitions look good. Verify no breaking changes.The simplification of
nightjarandkiwicolor definitions improves maintainability. However, ensure that removing thebgproperty doesn't break any existing styles.Please run the following script to check for any usage of the removed color properties:
If the script returns any results, consider keeping the
bgproperties or updating the affected styles.✅ Verification successful
Color definitions update verified. No breaking changes detected.
The removal of the
bgproperty from bothnightjarandkiwicolor definitions does not impact any existing styles. The simplification enhances maintainability without introducing issues.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for usage of removed color properties # Test: Search for nightjar bg and kiwi bg usage rg --type-add 'frontend:*.{cljs,cljc,clj,edn}' --type frontend 'nightjar.*bg|kiwi.*bg'Length of output: 828
Script:
#!/bin/bash # Description: Verify usage of removed color properties nightjar.bg and kiwi.bg # Search for usage of nightjar.bg and kiwi.bg in frontend files rg --type-add 'frontend:*.{cljs,cljc,clj,edn,js,jsx,ts,tsx}' --type frontend 'nightjar\.bg|kiwi\.bg'Length of output: 102
| content: [ | ||
| './src/mockingbird/**/*.cljs', | ||
| './public/**/*.html', | ||
| ], |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
LGTM! Consider using a more specific path for HTML files.
The change from purge to content is correct and aligns with Tailwind CSS v3.0+ requirements. The addition of HTML files to the content array is a good practice to ensure all used classes are included in the final build.
Consider using a more specific path for HTML files to avoid unnecessary scanning:
content: [
'./src/mockingbird/**/*.cljs',
- './public/**/*.html',
+ './public/*.html',
],This assumes your HTML files are directly in the public folder. Adjust if they are in subfolders.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| content: [ | |
| './src/mockingbird/**/*.cljs', | |
| './public/**/*.html', | |
| ], | |
| content: [ | |
| './src/mockingbird/**/*.cljs', | |
| './public/*.html', | |
| ], |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range comments (1)
src/mockingbird/helpers/props.cljs (1)
Line range hint
1-45: Summary: Changes align with PR objectives but require careful testingThe modifications to the
get-propsfunction in this file align well with the PR objectives of facilitating component releases and transitions. The addition ofwidthandheightparameters and the change in class concatenation order provide more flexibility for component sizing and styling, which can be beneficial during the transition process.However, to ensure a smooth transition and release of components:
- Implement the usage of
widthandheightparameters as suggested earlier.- Carefully test all components using
get-propsto ensure the new class concatenation order doesn't introduce unexpected styling issues.- Update any relevant documentation to reflect these changes.
These steps will help maintain consistency and prevent potential issues as components are transferred and released in the new mockingbird project structure.
Consider creating a test suite specifically for components using
get-propsto streamline the verification process during this transition period.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (4)
- src/mockingbird/components/button.cljs (1 hunks)
- src/mockingbird/components/input.cljs (1 hunks)
- src/mockingbird/helpers/props.cljs (2 hunks)
- test/unit/components/props_test.cljs (1 hunks)
🔇 Additional comments (12)
src/mockingbird/helpers/props.cljs (1)
45-45: 🛠️ Refactor suggestionConsider the impact of changing the class concatenation order
The order of string concatenation has been modified, moving
classto the beginning of the string:(str class actual-size actual-roundness cast-shadow actual-padding actual-margin)This change allows custom classes to take precedence over the default styles, which can provide more flexibility in component styling. However, it's important to consider the following:
- This change might affect existing components that rely on the previous order of class application.
- It could potentially override important default styles unintentionally.
To ensure this change doesn't introduce unexpected styling issues:
- Review and test affected components to verify that the new class order produces the desired visual results.
- Update documentation to reflect this change in behavior, if necessary.
- Consider adding a comment in the code explaining the rationale behind this change.
To identify potentially affected components, run the following script:
This will help you identify which components might need review or testing due to this change.
✅ Verification successful
Class Concatenation Order Change Affects Only
imageComponentThe modification to concatenate the
classat the beginning impacts only theimage.cljscomponent:src/mockingbird/components/image.cljs: (d/img {:class (str (get-propsRecommended Actions:
Review and Test
image.cljs:
- Verify that the new class order does not unintentionally override default styles.
- Ensure that the visual appearance of the image component remains consistent.
Update Documentation:
- If necessary, add comments explaining the rationale behind the class concatenation order change within the
imagecomponent.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Find all usages of get-props function in component files rg --type clojure 'get-props' src/mockingbird/componentsLength of output: 692
Script:
#!/bin/bash # Search for get-props usage within :class or class attributes in component files rg --type clojure '[:]?class\s*\(str\s*\(get-props' src/mockingbird/componentsLength of output: 154
test/unit/components/props_test.cljs (3)
Line range hint
1-57: Summary of changes and recommendations
- The reformatting of test cases improves readability.
- There's a consistent change in the output format, with custom classes now appearing at the beginning of the string. This change should be verified across the codebase.
- Trailing spaces have been introduced in various test cases and should be removed for consistency.
- The full-size output only includes "w-full", omitting "h-full". This should be verified if it's intentional.
- For complete consistency, consider adding trailing commas after the last properties in input maps across all test cases.
These changes maintain test coverage while improving code clarity. Please address the identified issues and verify the intentional changes in the
get-propsfunction implementation.
24-31: 🧹 Nitpick (assertive)Verify full-size output and remove trailing space.
The output for full size only includes "w-full", omitting "h-full". Please verify if this is intentional. Also, the trailing space issue is present in this test case as well.
Consider removing the trailing space and verifying the full-size output:
- :class "basic-class "}) + :class "basic-class"}) - "basic-class w-full ")) + "basic-class w-full"))To verify the full-size output:
#!/bin/bash # Description: Check the implementation of get-props for full size # Test: Search for the handling of :full size in get-props function rg --type clojure -e ':full.*:size' -A 10 --glob '!test/**'
8-15: 🧹 Nitpick (assertive)Verify output format change and remove trailing space.
The expected output format has changed, with the custom class now at the beginning. This might indicate a change in the
get-propsfunction implementation. Please ensure this change is intentional and consistent across the codebase.Also, there's a trailing space in both the input
:classvalue and the expected output. This is unusual and might be unintentional.Consider removing the trailing space:
- :class "custom-class "}) + :class "custom-class"}) - "custom-class w-8 h-8 rounded-sm shadow-sm p-2 m-2 ")) + "custom-class w-8 h-8 rounded-sm shadow-sm p-2 m-2"))To verify the change in output format:
✅ Verification successful
Output format and trailing space confirmed.
The
get-propsfunction intentionally appends a trailing space to the:classattribute and other properties to ensure proper CSS class concatenation. The change in output format, positioning the custom class at the beginning, is consistent with the function's implementation and is intentional.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistency in get-props output format across the codebase # Test: Search for get-props function calls and their expected outputs rg --type clojure -e '\(get-props' -A 10 --glob '!test/**'Length of output: 155
Script:
#!/bin/bash # Description: Check for consistency in get-props output format across the codebase # Test: Search for get-props function calls and their expected outputs rg --type clojure -e '\(get-props' -A 10 --glob '!test/**'Length of output: 3160
Script:
#!/bin/bash # Description: Locate the definition of the get-props function in the codebase # Search for the definition of get-props rg --type clojure -e '\(defn\s+get-props'Length of output: 94
Script:
#!/bin/bash # Description: Retrieve the implementation of the get-props function # Search for and display the get-props function definition and its implementation rg --type clojure -A 20 '\(defn\s+get-props' src/mockingbird/helpers/props.cljsLength of output: 670
Script:
#!/bin/bash # Description: Retrieve the full implementation of the get-props function # Search for and display the complete get-props function definition rg --type clojure -A 50 '\(defn\s+get-props' src/mockingbird/helpers/props.cljsLength of output: 1198
src/mockingbird/components/input.cljs (1)
19-52: LGTM: Enhancedinputcomponent with new customization optionsThe addition of
size,roundness,shadow,margin, andpaddingproperties increases the flexibility and reusability of theinputcomponent. The default values ensure backward compatibility.src/mockingbird/components/button.cljs (7)
4-4: Approve the addition of theget-propsimport.The
get-propsfunction is essential for generating additional class names based on the new parameters, enhancing the component's flexibility.
9-12: Consider addinginline-flexto the:submitstyle for consistency.The
:submitstyle includestransition-allbut lacksinline-flex, which is present in other styles like:defaultand:highlight. For consistency and to maintain uniform behavior across button styles, consider addinginline-flexto the:submitstyle.
14-18: Approve the function signature changes with enhanced customizability.Adding new parameters (
size,roundness,shadow,margin,padding,loading?,children) to thebuttoncomponent allows for greater customization and flexibility in button styling and behavior.
23-29: Approve the addition of default values for new parameters.Providing default values for the new parameters ensures that the component behaves predictably even when these options are not explicitly set, improving usability.
32-39: Validate the usage ofget-propsfor dynamic class generation.The integration of
get-propseffectively generates dynamic class names based on the provided parameters, streamlining the styling process.
42-46: Synchronize the displayed text andaria-labelwhen not loading.There is an inconsistency between the
aria-labeland the displayed text whenloading?isfalse. Thearia-labeluseslabel, while the displayed text useschildrenor defaults to "Insert some text". To enhance accessibility and consistency, update thearia-labelto match the displayed text.
42-42:⚠️ Potential issueCorrect the
typeattribute to match HTML specifications.The
:typeattribute for the HTMLbuttonelement should be either"button","submit", or"reset". Using custom values like:default,:highlight, or:submitmay cause unexpected behavior.Apply this diff to set the correct
typeattribute:- :type type + :type (case type + :submit "submit" + "button")This change ensures that the
typeattribute is"submit"whentypeis:submit; otherwise, it defaults to"button".Likely invalid or redundant comment.
| [{:keys [size width height | ||
| roundness shadow | ||
| margin padding class] | ||
| :or {size :md roundness :none shadow :none padding :none margin :none}}] | ||
| :or {size :md width nil height nil roundness :none shadow :none padding :none margin :none}}] |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Implement usage of width and height parameters
The width and height parameters have been added to the function signature, which is a good step towards more flexible component sizing. However, these parameters are currently not used within the function body.
Consider the following suggestions:
- Implement logic to use
widthandheightwhen provided, overriding thesizeparameter. - Update the
actual-sizecalculation to incorporatewidthandheightwhen available.
Example implementation:
(let [actual-size
(cond
(and width height) (str "w-[" width "] h-[" height "] ")
:else
(case size
:none ""
:sm "w-8 h-8 "
:md "w-16 h-16 "
:lg "w-32 h-32 "
:xl "w-64 h-64 "
:full "w-full "
"w-16 h-16 "))]
...)This change will allow for more precise control over component dimensions when needed.
Would you like assistance in implementing this change?
| (is (= (get-props | ||
| {:size :xl | ||
| :roundness :none | ||
| :shadow :none}) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove trailing space for consistency.
This test case is consistent with the previous ones in terms of the new output format. The trailing space issue is present here as well, but at the end of the output string.
Consider removing the trailing space:
- "w-64 h-64 "))))
+ "w-64 h-64"))))Committable suggestion was skipped due to low confidence.
| (is (= (get-props | ||
| {:size :lg | ||
| :roundness :full | ||
| :shadow :lg | ||
| :margin :md | ||
| :padding :lg | ||
| :class "another-class "}) | ||
| "another-class w-32 h-32 rounded-full shadow-lg p-8 m-4 ")) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove trailing space for consistency.
This test case is consistent with the previous one in terms of the new output format. However, the trailing space issue is present here as well.
Consider removing the trailing space:
- :class "another-class "})
+ :class "another-class"})
- "another-class w-32 h-32 rounded-full shadow-lg p-8 m-4 "))
+ "another-class w-32 h-32 rounded-full shadow-lg p-8 m-4"))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| (is (= (get-props | |
| {:size :lg | |
| :roundness :full | |
| :shadow :lg | |
| :margin :md | |
| :padding :lg | |
| :class "another-class "}) | |
| "another-class w-32 h-32 rounded-full shadow-lg p-8 m-4 ")) | |
| (is (= (get-props | |
| {:size :lg | |
| :roundness :full | |
| :shadow :lg | |
| :margin :md | |
| :padding :lg | |
| :class "another-class"}) | |
| "another-class w-32 h-32 rounded-full shadow-lg p-8 m-4")) |
|
|
||
| (testing "Partial defaults" | ||
| (is (= (get-props {:size :sm}) | ||
| (is (= (get-props | ||
| {:size :sm}) | ||
| "w-8 h-8 ")) | ||
| (is (= (get-props {:roundness :full | ||
| :shadow :lg}) | ||
| (is (= (get-props | ||
| {:roundness :full | ||
| :shadow :lg}) | ||
| "w-16 h-16 rounded-full shadow-lg "))) | ||
|
|
||
| (testing "Nil values" | ||
| (is (= (get-props {:size nil | ||
| :roundness nil}) | ||
| (is (= (get-props | ||
| {:size nil | ||
| :roundness nil}) | ||
| "w-16 h-16 "))))) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add trailing commas for consistency.
The formatting changes in the test-default-cases function improve readability. For complete consistency with other test cases, consider adding trailing commas after the last properties in the input maps.
Consider applying these minor changes:
(is (= (get-props
- {:size :sm})
+ {:size :sm,})
"w-8 h-8 "))
(is (= (get-props
{:roundness :full
- :shadow :lg})
+ :shadow :lg,})
"w-16 h-16 rounded-full shadow-lg ")))
(testing "Nil values"
(is (= (get-props
{:size nil
- :roundness nil})
+ :roundness nil,})
"w-16 h-16 ")))))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| (testing "Partial defaults" | |
| (is (= (get-props {:size :sm}) | |
| (is (= (get-props | |
| {:size :sm}) | |
| "w-8 h-8 ")) | |
| (is (= (get-props {:roundness :full | |
| :shadow :lg}) | |
| (is (= (get-props | |
| {:roundness :full | |
| :shadow :lg}) | |
| "w-16 h-16 rounded-full shadow-lg "))) | |
| (testing "Nil values" | |
| (is (= (get-props {:size nil | |
| :roundness nil}) | |
| (is (= (get-props | |
| {:size nil | |
| :roundness nil}) | |
| "w-16 h-16 "))))) | |
| (testing "Partial defaults" | |
| (is (= (get-props | |
| {:size :sm,}) | |
| "w-8 h-8 ")) | |
| (is (= (get-props | |
| {:roundness :full | |
| :shadow :lg,}) | |
| "w-16 h-16 rounded-full shadow-lg "))) | |
| (testing "Nil values" | |
| (is (= (get-props | |
| {:size nil | |
| :roundness nil,}) | |
| "w-16 h-16 "))))) |
| :margin margin | ||
| :padding padding | ||
| :class class})) | ||
| :on-click (when-not disabled on-click) |
There was a problem hiding this comment.
Disable on-click handler when loading? is true.
Currently, the :on-click handler is disabled only when disabled is true. When loading? is true, the button appears disabled, but the :on-click handler may still be active, potentially leading to unintended interactions.
Apply this diff to disable on-click when loading? is true:
- :on-click (when-not disabled on-click)
+ :on-click (when-not (or disabled loading?) on-click)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| :on-click (when-not disabled on-click) | |
| :on-click (when-not (or disabled loading?) on-click) |
There was a problem hiding this comment.
Actionable comments posted: 11
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (5)
- src/mockingbird/components/button.cljs (1 hunks)
- src/mockingbird/components/image.cljs (1 hunks)
- src/mockingbird/components/message.cljs (1 hunks)
- src/mockingbird/components/pfp.cljs (1 hunks)
- src/mockingbird/examples/main.cljs (1 hunks)
🔇 Additional comments (10)
src/mockingbird/examples/main.cljs (3)
3-12: LGTM: Namespace imports updated correctlyThe namespace declaration has been updated with the necessary imports for the new components and libraries used in the
appfunction. This change aligns well with the modifications made to the main application structure.
18-18: Verify logo file existence and update path if necessaryThe addition of the
:logoproperty to thenav-barcomponent is a good enhancement. However, as mentioned in a previous review, please ensure that thelogo.pngfile exists in thepublic/imagesdirectory. If the file is missing or the path has changed, update it accordingly.Run the following script to verify the logo file's existence and its references:
#!/bin/bash # Verify the existence of the logo file if fd -t f "logo.png" public/images; then echo "Logo file found in public/images directory." else echo "Warning: logo.png not found in public/images directory." echo "Please ensure the logo file is present in the correct location." fi # Check for other references to the logo path in the codebase rg "/images/logo.png" --type clojure
16-16: 🧹 Nitpick (assertive)Address the TODO comment for component demonstration
The TODO comment indicates the need to create an example that demonstrates the components' capabilities. This aligns with the PR objectives of iterating on components and preparing for release.
Would you like assistance in creating this example, or should we create a GitHub issue to track this task for future implementation?
src/mockingbird/components/pfp.cljs (1)
13-42:pfpcomponent is well-structured and follows best practicesThe
pfpcomponent is properly defined with clear prop destructuring and sensible default values. The use ofget-propsto handle dynamic class generation based on props enhances readability and maintainability.src/mockingbird/components/button.cljs (2)
4-4: Approve the addition of theget-propsdependencyThe inclusion of
[mockingbird.helpers.props :refer [get-props]]in the:requiresection correctly imports theget-propsfunction. This function is essential for generating additional class names based on component properties, enhancing the flexibility and customization of thebuttoncomponent.
19-31: Verify the usage of the updatedbuttoncomponent across the codebaseWith the addition of new parameters (
size,roundness,shadow,margin,padding,loading?,children), it's important to ensure that all usages of thebuttoncomponent are updated accordingly. This will prevent unexpected behavior or runtime errors due to missing or mismatched props.Run the following script to identify all usages of the
buttoncomponent:Review the results to confirm that all instances pass the necessary props or adjust them as needed based on the new parameters.
src/mockingbird/components/message.cljs (1)
10-16: Code is correct and well-implementedThe
gen-today-datefunction accurately generates the current date in "DD/MM/YYYY" format, correctly handling zero-padding and month incrementation.src/mockingbird/components/image.cljs (3)
15-15: Makealtparameter required to improve accessibilityPrevious reviews have suggested making the
altparameter required instead of providing a default value. This encourages better accessibility practices by ensuring that each image has meaningful alternative text.
24-34: Consider adding error handling for image loading failuresTo improve robustness, consider adding an
onErrorhandler to manage cases where the image fails to load. This was suggested in previous reviews and is still applicable.
8-8: Consider memoizing the component for performance optimizationAs suggested in previous reviews, memoizing the component using
helix/memocan prevent unnecessary re-renders when props haven't changed, enhancing performance.
| ($ main | ||
| ($ message | ||
| {:title "Welcome to moclojer beta" | ||
| :email "[email protected]" | ||
| :author "Avelino" | ||
| :image "/images/logo.png" | ||
| :image-style "rounded" | ||
| :message ["I'm happy to have you here, we are launching the SaaS version of moclojer, seeking to make it simple for you to put a mock API in the air to develop your projects." "I look forward to your feedback (this is the only way we can improve the product)."]})) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
LGTM: Enhanced main content with message component
The addition of the message component significantly improves the content display in the main section. This change aligns well with the PR objectives of transitioning and enhancing components for the first release.
A minor suggestion for improvement:
Consider extracting the message content into a separate constant or configuration file. This would make it easier to update the content in the future and potentially support internationalization.
Example:
(def welcome-message
{:title "Welcome to moclojer beta"
:email "[email protected]"
:author "Avelino"
:image "/images/logo.png"
:image-style "rounded"
:message ["I'm happy to have you here, we are launching the SaaS version of moclojer, seeking to make it simple for you to put a mock API in the air to develop your projects."
"I look forward to your feedback (this is the only way we can improve the product)."]})
;; In the app function
($ message welcome-message)| [helix.dom :as d])) | ||
|
|
||
| (def pfp-styles | ||
| {:mockinbird {:default "w-8 h-8 rounded-none opacity-100" |
There was a problem hiding this comment.
Fix typo in keyword :mockinbird to ensure correct style application
The keyword :mockinbird appears to be misspelled in both the pfp-styles map and the default value for theme. It should be :mockingbird to match the project name and ensure styles are correctly applied.
Apply this diff to correct the typo:
(def pfp-styles
- {:mockinbird {:default "w-8 h-8 rounded-none opacity-100"
+ {:mockingbird {:default "w-8 h-8 rounded-none opacity-100"
:rounded "w-8 h-8 rounded-full opacity-100"
:loading "w-8 h-8 rounded-full opacity-30 animate-pulse"}})
...
:or {
- theme :mockinbird
+ theme :mockingbird
style :default
src "/images/logo.png"
alt "Profile picture"Also applies to: 18-18
| (if loading? | ||
| "Loading..." | ||
| (or children "Insert some text"))))) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
🛠️ Refactor suggestion
Enhance the loading state with a visual indicator
Currently, when loading? is true, the button displays the text "Loading...". To improve user experience, consider adding a visual indicator, such as a spinner or progress bar, alongside the loading text to clearly convey the loading state.
You could update the rendering logic as follows:
(if loading?
(d/div
{:class "flex items-center"}
(d/span
{:class "loading-spinner mr-2"}) ; Ensure you have corresponding CSS for the spinner
(d/span "Loading..."))
(or children "Insert some text"))This change adds a spinner next to the loading text, providing a clearer visual cue to users.
| :aria-label (if loading? "Loading..." label)} | ||
| (if loading? | ||
| "Loading..." | ||
| (or children "Insert some text"))))) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
🛠️ Refactor suggestion
Consider providing a more user-friendly default text or requiring children
The default text "Insert some text" may not be informative or appropriate in a production environment. Consider providing a more meaningful default text like "Click me", or make the children prop mandatory to ensure that the button always displays proper content.
You could adjust the default as follows:
- (or children "Insert some text")
+ (or children "Click me")Alternatively, you could enforce the presence of children:
(when-not children
(throw (js/Error "The 'children' prop is required for the 'button' component.")))This ensures that the button always has intentional and meaningful content.
| [{:keys [image children author title message email] | ||
| :or {image "" | ||
| author "Team Mockingbird"}}] |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove unused 'children' prop in 'message' component
The children prop is included in the component's argument destructuring but isn't used within the message component. Consider removing it to clean up the code.
Apply this diff to remove the unused children prop:
- [{:keys [image children author title message email]
+ [{:keys [image author title message email]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [{:keys [image children author title message email] | |
| :or {image "" | |
| author "Team Mockingbird"}}] | |
| [{:keys [image author title message email] | |
| :or {image "" | |
| author "Team Mockingbird"}}] |
| (do | ||
| (d/br) | ||
| ($ text {:message message :index (inc index)})))))) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Simplify code by removing unnecessary 'do' block
In the when expression, the do block is unnecessary since it wraps only expressions that return React elements. You can simplify the code by removing the do.
Here's the suggested change:
- (when (< index (dec (count message)))
- (do
- (d/br)
- ($ text {:message message :index (inc index)})))
+ (when (< index (dec (count message)))
+ (d/br)
+ ($ text {:message message :index (inc index)}))Committable suggestion was skipped due to low confidence.
| (defnc text | ||
| [{:keys [message index]}] | ||
| (let [msg-seq (seq message)] | ||
| (d/div | ||
| (d/p {:class "text-base font-normal text-gray-500 dark:text-gray-400"} | ||
| (nth msg-seq index)) | ||
| (when (< index (dec (count message))) | ||
| (do | ||
| (d/br) | ||
| ($ text {:message message :index (inc index)})))))) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor 'text' component to avoid recursion and improve performance
The current implementation of the text component uses recursion to render each message, which might lead to stack overflows or performance issues with long message lists. Consider refactoring it to use higher-order functions like map or for for better readability and efficiency.
Here's a suggested refactor:
- (defnc text
- [{:keys [message index]}]
- (let [msg-seq (seq message)]
- (d/div
- (d/p {:class "text-base font-normal text-gray-500 dark:text-gray-400"}
- (nth msg-seq index))
- (when (< index (dec (count message)))
- (do
- (d/br)
- ($ text {:message message :index (inc index)}))))))
+ (defnc text
+ [{:keys [message]}]
+ (d/div
+ (for [msg message]
+ (d/div
+ (d/p {:class "text-base font-normal text-gray-500 dark:text-gray-400"}
+ msg)
+ (d/br)))))You'll also need to update the call to text in the message component:
- ($ text {:message (seq message) :index 0})
+ ($ text {:message message})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| (defnc text | |
| [{:keys [message index]}] | |
| (let [msg-seq (seq message)] | |
| (d/div | |
| (d/p {:class "text-base font-normal text-gray-500 dark:text-gray-400"} | |
| (nth msg-seq index)) | |
| (when (< index (dec (count message))) | |
| (do | |
| (d/br) | |
| ($ text {:message message :index (inc index)})))))) | |
| (defnc text | |
| [{:keys [message]}] | |
| (d/div | |
| (for [msg message] | |
| (d/div | |
| (d/p {:class "text-base font-normal text-gray-500 dark:text-gray-400"} | |
| msg) | |
| (d/br))))) |
| (d/p {:class "p-2 text-base font-normal text-gray-500 dark:text-gray-400"} | ||
| "Best Regards," | ||
| (d/br) | ||
| (d/i "Avelino Founder Team")))))) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider parameterizing the closing signature
The closing signature "Best Regards," and "Avelino Founder Team" are hardcoded. Consider turning these into props to make the component more reusable and customizable.
Here's how you might adjust the code:
+ [{:keys [image author title message email closing-text closing-signature]
+ :or {image ""
+ author "Team Mockingbird"
+ closing-text "Best Regards,"
+ closing-signature "Avelino Founder Team"}}]
...
- (d/p {:class "p-2 text-base font-normal text-gray-500 dark:text-gray-400"}
- "Best Regards,"
- (d/br)
- (d/i "Avelino Founder Team"))
+ (d/p {:class "p-2 text-base font-normal text-gray-500 dark:text-gray-400"}
+ closing-text
+ (d/br)
+ (d/i closing-signature))Committable suggestion was skipped due to low confidence.
| :or {theme :mockingbird | ||
| image "/images/logo.png" | ||
| alt "test" | ||
| src "/images/logo.png" |
There was a problem hiding this comment.
Make src parameter required to ensure correct image loading
Providing a default src value of "/images/logo.png" may lead to unintended images being displayed when no src is specified. It's better to make src a required parameter to ensure that the component always displays the intended image.
Apply this diff to remove the default src value:
:or {
- src "/images/logo.png"
+ src nil
alt "Image description"
size :md
roundness :none
shadow :none
margin :none
padding :none
loading? false
children nil}}]Add a runtime check to ensure src is provided:
(when (nil? src)
(throw (js/Error. "Image `src` parameter is required.")))| (defnc ^:export image | ||
| [{:keys [class theme src | ||
| alt size roundness | ||
| shadow margin padding | ||
| children] | ||
| loading? children] | ||
| :or {theme :mockingbird | ||
| image "/images/logo.png" | ||
| alt "test" | ||
| src "/images/logo.png" | ||
| alt "Image description" | ||
| size :md | ||
| roundness :none | ||
| shadow :none | ||
| margin :none | ||
| padding :none}}] | ||
| padding :none | ||
| loading? false | ||
| children nil}}] |
There was a problem hiding this comment.
Remove unused theme parameter to simplify the component interface
The theme parameter is included in the component's parameters with a default value of :mockingbird, but it is not used anywhere within the component. Unused parameters can lead to confusion and should be removed to keep the component interface clean.
Apply this diff to remove the unused theme parameter:
- [{:keys [class theme src
+ [{:keys [class src
alt size roundness
shadow margin padding
loading? children]
:or {
- theme :mockingbird
src "/images/logo.png"
alt "Image description"
size :md
roundness :none
shadow :none
margin :none
padding :none
loading? false
children nil}}]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| (defnc ^:export image | |
| [{:keys [class theme src | |
| alt size roundness | |
| shadow margin padding | |
| children] | |
| loading? children] | |
| :or {theme :mockingbird | |
| image "/images/logo.png" | |
| alt "test" | |
| src "/images/logo.png" | |
| alt "Image description" | |
| size :md | |
| roundness :none | |
| shadow :none | |
| margin :none | |
| padding :none}}] | |
| padding :none | |
| loading? false | |
| children nil}}] | |
| (defnc ^:export image | |
| [{:keys [class src | |
| alt size roundness | |
| shadow margin padding | |
| loading? children] | |
| :or {src "/images/logo.png" | |
| alt "Image description" | |
| size :md | |
| roundness :none | |
| shadow :none | |
| margin :none | |
| padding :none | |
| loading? false | |
| children nil}}] |
fixes - npm compiling on clojar for css generation - building names
There was a problem hiding this comment.
Actionable comments posted: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (2)
- .github/workflows/npm.yml (1 hunks)
- .github/workflows/release.yml (1 hunks)
🧰 Additional context used
🪛 yamllint
.github/workflows/npm.yml
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 5-5: wrong indentation: expected 8 but found 6
(indentation)
[error] 6-6: wrong indentation: expected 10 but found 8
(indentation)
[error] 17-17: wrong indentation: expected 18 but found 16
(indentation)
[error] 24-24: wrong indentation: expected 18 but found 16
(indentation)
.github/workflows/release.yml
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 5-5: wrong indentation: expected 8 but found 6
(indentation)
[error] 6-6: wrong indentation: expected 10 but found 8
(indentation)
[error] 20-20: wrong indentation: expected 18 but found 16
(indentation)
[error] 23-23: wrong indentation: expected 18 but found 16
(indentation)
[error] 26-26: wrong indentation: expected 18 but found 16
(indentation)
[error] 33-33: trailing spaces
(trailing-spaces)
🔇 Additional comments (6)
.github/workflows/npm.yml (4)
1-7: LGTM: Workflow name and trigger events are well-defined.The workflow name "Npm-Release" is descriptive, and the trigger events (tag push and manual dispatch) are appropriate for an npm release workflow.
🧰 Tools
🪛 yamllint
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 5-5: wrong indentation: expected 8 but found 6
(indentation)
[error] 6-6: wrong indentation: expected 10 but found 8
(indentation)
9-11: LGTM: Job definition and runner are correctly specified.The 'publish' job is well-defined and set to run on the latest Ubuntu runner, which is suitable for an npm release workflow.
1-24: Overall workflow structure is good, with minor improvements needed.The npm release workflow is well-defined and includes the necessary steps for checking out the code, setting up the environment, building the package, and publishing to npm. The suggested changes will improve the workflow's clarity and efficiency. Please address the following points:
- Remove the unnecessary Clojure setup step.
- Fix indentation issues in the Node.js setup and npm publish steps.
- Improve the build step name and remove the unnecessary backslash.
After making these changes, the workflow should be ready for use.
🧰 Tools
🪛 yamllint
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 5-5: wrong indentation: expected 8 but found 6
(indentation)
[error] 6-6: wrong indentation: expected 10 but found 8
(indentation)
[error] 17-17: wrong indentation: expected 18 but found 16
(indentation)
[error] 24-24: wrong indentation: expected 18 but found 16
(indentation)
18-21:⚠️ Potential issueImprove build step name and remove unnecessary backslash.
- Provide a more descriptive name for the build step.
- Remove the unnecessary backslash at the end of the first command.
Apply this diff to address these issues:
-- name: npm release +- name: Build and package the library run: | - npm ci && \ + npm ci && npm run lib.github/workflows/release.yml (2)
1-7: LGTM! The workflow name and triggers are well-defined.The workflow name "Release" is clear and descriptive. The triggers (push events with tags matching 'v*' and manual workflow_dispatch) are appropriate for a release workflow, allowing for both automated and manual releases.
🧰 Tools
🪛 yamllint
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 5-5: wrong indentation: expected 8 but found 6
(indentation)
[error] 6-6: wrong indentation: expected 10 but found 8
(indentation)
9-15: LGTM! The job configuration is well-structured.The job configuration follows best practices:
- Running on the latest Ubuntu environment ensures up-to-date tools.
- Using a specific environment ("env") is good for managing secrets.
- Setting the default shell to bash ensures consistency across steps.
There was a problem hiding this comment.
Actionable comments posted: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (2)
- .github/workflows/npm.yml (1 hunks)
- .github/workflows/release.yml (1 hunks)
🧰 Additional context used
🪛 yamllint
.github/workflows/npm.yml
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 5-5: wrong indentation: expected 8 but found 6
(indentation)
[error] 6-6: wrong indentation: expected 10 but found 8
(indentation)
[error] 17-17: wrong indentation: expected 18 but found 16
(indentation)
[error] 24-24: wrong indentation: expected 18 but found 16
(indentation)
[warning] 25-25: comment not indented like content
(comments-indentation)
.github/workflows/release.yml
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 5-5: wrong indentation: expected 8 but found 6
(indentation)
[error] 6-6: wrong indentation: expected 10 but found 8
(indentation)
[error] 20-20: wrong indentation: expected 18 but found 16
(indentation)
[error] 23-23: wrong indentation: expected 18 but found 16
(indentation)
[error] 26-26: wrong indentation: expected 18 but found 16
(indentation)
🔇 Additional comments (4)
.github/workflows/npm.yml (4)
1-7: LGTM: Workflow name and triggers are well-defined.The workflow name "Npm-Release" clearly indicates its purpose, and the trigger events (tag push and manual dispatch) are appropriate for managing npm releases.
🧰 Tools
🪛 yamllint
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 5-5: wrong indentation: expected 8 but found 6
(indentation)
[error] 6-6: wrong indentation: expected 10 but found 8
(indentation)
13-17:⚠️ Potential issueRemove unnecessary Clojure setup and fix indentation.
The
DeLaGuardo/[email protected]action seems unnecessary for an npm release workflow. Consider removing it unless there's a specific need for Clojure in this context.Fix the indentation of the
withsection under the Node.js setup step.Apply this diff to address these issues:
- uses: actions/checkout@v4 -- uses: DeLaGuardo/[email protected] - uses: actions/setup-node@v3 with: - node-version: "20" + node-version: "20"🧰 Tools
🪛 yamllint
[error] 17-17: wrong indentation: expected 18 but found 16
(indentation)
18-21: 🧹 Nitpick (assertive)Improve build step naming and remove unnecessary backslash.
- Provide a descriptive name for the build step to improve readability in the workflow logs.
- Remove the unnecessary backslash at the end of the 'npm ci' command.
Apply this diff to address these issues:
- - name: npm release + - name: Build and package the library run: | - npm ci && \ + npm ci && npm run lib
22-24: 🧹 Nitpick (assertive)Fix indentation in npm publish step.
The
withsection under the npm publish step is not properly indented.Apply this diff to fix the indentation:
- uses: JS-DevTools/[email protected] with: - token: ${{ secrets.NPM_TOKEN }} + token: ${{ secrets.NPM_TOKEN }}🧰 Tools
🪛 yamllint
[error] 24-24: wrong indentation: expected 18 but found 16
(indentation)
| - uses: JS-DevTools/[email protected] | ||
| with: | ||
| token: ${{ secrets.NPM_TOKEN }} | ||
| # TODO |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Address or remove the TODO comment.
There's a TODO comment at the end of the file. Please either address the TODO item or remove the comment if it's no longer relevant.
If there are no additional steps needed, you can simply remove the TODO comment:
- # TODO📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # TODO |
🧰 Tools
🪛 yamllint
[warning] 25-25: comment not indented like content
(comments-indentation)
| name: Release | ||
|
|
||
| on: | ||
| push: | ||
| tags: | ||
| - 'v*' | ||
| workflow_dispatch: |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
LGTM! The workflow name and triggers are well-defined.
The workflow name "Release" is clear and descriptive. The triggers are appropriate for a release workflow, allowing for both automated releases on version tags and manual triggering.
Consider adding a brief description comment at the beginning of the file to provide more context:
# Workflow for releasing the project to Clojars
# Triggered on pushing tags starting with 'v' or manually
name: Release🧰 Tools
🪛 yamllint
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 5-5: wrong indentation: expected 8 but found 6
(indentation)
[error] 6-6: wrong indentation: expected 10 but found 8
(indentation)
| jobs: | ||
| clojars: | ||
| runs-on: ubuntu-latest | ||
| environment: env | ||
| defaults: | ||
| run: | ||
| shell: bash |
There was a problem hiding this comment.
Job setup looks good, but there are some indentation issues.
The job setup is appropriate for a Clojure release workflow. Using a specific environment is good for managing secrets.
There are some indentation inconsistencies. Please adjust the indentation as follows:
jobs:
clojars:
runs-on: ubuntu-latest
environment: env
defaults:
run:
shell: bashThis will resolve the yamllint warnings and improve readability.
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - uses: DeLaGuardo/[email protected] | ||
| with: | ||
| cli: "1.11.1.1139" | ||
| - uses: actions/setup-node@v3 | ||
| with: | ||
| node-version: "20" |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Environment setup is comprehensive, but there are indentation issues.
The setup steps for Git, Clojure, and Node.js are comprehensive and use recent versions, which is good for security and accessing the latest features.
Please correct the indentation for the with sections:
steps:
- uses: actions/checkout@v4
- uses: DeLaGuardo/[email protected]
with:
cli: "1.11.1.1139"
- uses: actions/setup-node@v3
with:
node-version: "20"Consider adding a comment explaining why this specific Clojure CLI version (1.11.1.1139) is used. This will help future maintainers understand if and when it needs to be updated.
🧰 Tools
🪛 yamllint
[error] 20-20: wrong indentation: expected 18 but found 16
(indentation)
[error] 23-23: wrong indentation: expected 18 but found 16
(indentation)
| - name: Clojars release | ||
| env: | ||
| CLOJARS_USERNAME: ${{ secrets.CLOJARS_USERNAME }} | ||
| CLOJARS_PASSWORD: ${{ secrets.CLOJARS_PASSWORD }} | ||
| run: | | ||
| npm ci && \ | ||
| npm run release && \ | ||
| clojure -T:dev:build clean && \ | ||
| clojure -T:dev:build jar && \ | ||
| clojure -X:deploy-clojars |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Release step is secure and comprehensive, but could be improved.
The release process covers all necessary steps and uses GitHub secrets for Clojars credentials, which is a good security practice.
Correct the indentation of the env section:
- name: Clojars release
env:
CLOJARS_USERNAME: ${{ secrets.CLOJARS_USERNAME }}
CLOJARS_PASSWORD: ${{ secrets.CLOJARS_PASSWORD }}
run: |
npm ci && \
npm run release && \
clojure -T:dev:build clean && \
clojure -T:dev:build jar && \
clojure -X:deploy-clojarsConsider adding error handling and logging to make troubleshooting easier:
run: |
set -e
echo "Starting release process..."
npm ci
echo "Running release script..."
npm run release
echo "Cleaning build..."
clojure -T:dev:build clean
echo "Building JAR..."
clojure -T:dev:build jar
echo "Deploying to Clojars..."
clojure -X:deploy-clojars
echo "Release process completed successfully."This will provide more visibility into the release process and make it easier to identify where any failures occur.
🧰 Tools
🪛 yamllint
[error] 26-26: wrong indentation: expected 18 but found 16
(indentation)
| name: Release | ||
|
|
||
| on: | ||
| push: | ||
| tags: | ||
| - 'v*' | ||
| workflow_dispatch: | ||
|
|
||
| jobs: | ||
| clojars: | ||
| runs-on: ubuntu-latest | ||
| environment: env | ||
| defaults: | ||
| run: | ||
| shell: bash | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - uses: DeLaGuardo/[email protected] | ||
| with: | ||
| cli: "1.11.1.1139" | ||
| - uses: actions/setup-node@v3 | ||
| with: | ||
| node-version: "20" | ||
| - name: Clojars release | ||
| env: | ||
| CLOJARS_USERNAME: ${{ secrets.CLOJARS_USERNAME }} | ||
| CLOJARS_PASSWORD: ${{ secrets.CLOJARS_PASSWORD }} | ||
| run: | | ||
| npm ci && \ | ||
| npm run release && \ | ||
| clojure -T:dev:build clean && \ | ||
| clojure -T:dev:build jar && \ | ||
| clojure -X:deploy-clojars |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Overall, the release workflow is well-designed and aligns with the PR objectives.
This workflow effectively facilitates the release process for components that have been transferred from a private repository to the mockingbird project. It covers all necessary steps from building to deployment and uses secure practices for handling credentials.
The workflow aligns well with the PR objectives mentioned in issue #6, as it provides an automated way to release the components that have already been transferred. This will significantly aid in completing the transition of components and ensure that releases can be managed efficiently.
As you continue to transfer and implement additional components, consider the following:
- Ensure that all necessary build and test steps for new components are incorporated into this workflow.
- If different components require different release processes, consider using conditional steps or creating separate workflows for different component types.
- Implement a versioning strategy that aligns with your component transfer and release schedule.
These considerations will help maintain the efficiency of your release process as the project grows and evolves.
🧰 Tools
🪛 yamllint
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 5-5: wrong indentation: expected 8 but found 6
(indentation)
[error] 6-6: wrong indentation: expected 10 but found 8
(indentation)
[error] 20-20: wrong indentation: expected 18 but found 16
(indentation)
[error] 23-23: wrong indentation: expected 18 but found 16
(indentation)
[error] 26-26: wrong indentation: expected 18 but found 16
(indentation)
There was a problem hiding this comment.
Actionable comments posted: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (3)
- README.md (2 hunks)
- docs/Roadmap.md (0 hunks)
- docs/test.md (2 hunks)
💤 Files with no reviewable changes (1)
- docs/Roadmap.md
🧰 Additional context used
🪛 LanguageTool
README.md
[typographical] ~7-~7: Consider adding two commas here.
Context: ...ut ClojureScript (how dare you!) you can of course use our npm package into your JS applic...(OF_COURSE_COMMA)
[uncategorized] ~21-~21: The preposition “of” seems more likely in this position.
Context: ...ilities that increases speed and beauty on your UI development. By integrating Tai...(AI_EN_LECTOR_REPLACEMENT_PREPOSITION)
[style] ~21-~21: Consider a shorter alternative to avoid wordiness.
Context: ...gramming and a powerful component model in order to create a fast and lightweight design sy...(IN_ORDER_TO_PREMIUM)
[misspelling] ~56-~56: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...s, into your own config. # Usage add a import on a specific component or layou...(EN_A_VS_AN)
docs/test.md
[uncategorized] ~3-~3: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...everal times and also contribute to the open source code, won't you? > To do so, follow th...(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 Markdownlint
README.md
7-7: Expected: #installation; Actual: #Installation
Link fragments should be valid(MD051, link-fragments)
54-54: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
79-79: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
69-69: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
47-47: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
48-48: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
23-23: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
32-32: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
54-54: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
79-79: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
37-37: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
76-76: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
28-28: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
🔇 Additional comments (4)
docs/test.md (3)
3-3: Improved clarity and assertiveness in the introduction.The change from "you are probably gonna test it several times" to "you will test it several times" enhances the tone and clarity of the statement. This modification aligns well with the documentation's purpose of encouraging thorough testing and contribution.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~3-~3: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...everal times and also contribute to the open source code, won't you? > To do so, follow th...(EN_COMPOUND_ADJECTIVE_INTERNAL)
5-5: Clarified the nature of the document.Changing "follow this simple md file" to "follow this simple tutorial" provides a clearer description of what the reader can expect. This improvement helps set the right expectations for the user.
15-15: Updated npm command for Watch Mode.The command for Watch Mode has been simplified from
npm run app:watchtonpm run watch. This change suggests a modification in the project's npm scripts.To ensure consistency and prevent potential issues, please verify the following:
- The
package.jsonfile has been updated to reflect this change in the scripts section.- Any other documentation or files referencing the old command have been updated.
You can use the following script to check for any remaining occurrences of the old command:
Please review the results and update any remaining occurrences if necessary.
✅ Verification successful
Watch Command Update Verified
All references to
npm run app:watchhave been successfully removed, and thewatchscript inpackage.jsonhas been correctly updated torun-p shadow:watch-css shadow:watch-cljs.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for occurrences of the old command echo "Searching for occurrences of 'npm run app:watch':" rg --type-add 'npm:package.json' --type npm --type md --type js --type ts "npm run app:watch" echo "Verifying 'watch' script in package.json:" jq '.scripts.watch' package.jsonLength of output: 383
README.md (1)
29-30: 🧹 Nitpick (assertive)Approve the added links with a minor formatting suggestion.
The addition of direct links to the Node.js and npm installation pages is helpful for users. To improve readability, consider adding a blank line before the list:
Prerequisites Before you begin, ensure you have the following installed: - Clojure: Follow the [Clojure Installation guide](https://clojure.org/guides/install_clojure) to set up your environment. + - [Node](https://nodejs.org/en/download/prebuilt-installer/current). - [npm](https://www.npmjs.com/package/downloads).Likely invalid or redundant comment.
| @@ -1,8 +1,8 @@ | |||
| # Running the local mocking bird tests | |||
|
|
|||
| Mockingbird is a powerful tool for helping you build your project, but, as a great dev, you will not insert any random tool someone told you to. Instead, you are probably gonna test it several times and also contribute to the open source code, won't you? | |||
| Mockingbird is a powerful tool for helping you build your project, but, as a great dev, you will not insert any random tool someone told you to. Instead, you will test it several times and also contribute to the open source code, won't you? | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider hyphenating "open source" when used as a compound adjective.
The static analysis tool suggests using a hyphen in "open source" when it's used as a compound adjective. In this context, it's debatable whether it's necessary, as "open source" is commonly used without a hyphen.
If you decide to follow this suggestion, you could change the phrase to:
-...contribute to the open source code, won't you?
+...contribute to the open-source code, won't you?However, this change is optional and depends on your preferred style guide.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Mockingbird is a powerful tool for helping you build your project, but, as a great dev, you will not insert any random tool someone told you to. Instead, you will test it several times and also contribute to the open source code, won't you? | |
| Mockingbird is a powerful tool for helping you build your project, but, as a great dev, you will not insert any random tool someone told you to. Instead, you will test it several times and also contribute to the open-source code, won't you? |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~3-~3: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...everal times and also contribute to the open source code, won't you? > To do so, follow th...(EN_COMPOUND_ADJECTIVE_INTERNAL)
There was a problem hiding this comment.
there is no need to do this, @coderabbitai
There was a problem hiding this comment.
@Felipe-gsilva, understood. We'll refrain from suggesting this change in the future.
😔🐰
✏️ Learnings added
Learnt from: Felipe-gsilva
PR: moclojer/mockingbird#7
File: docs/test.md:3-3
Timestamp: 2024-10-04T17:06:28.658Z
Learning: In this project, it's acceptable to use "open source" without hyphenation, even when used as a compound adjective.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
README.md
Outdated
| We distribute our software via clojars (and intend to do so also by npm): | ||
|
|
||
| deps.edn: | ||
| ``` clj | ||
| moclojer/mockingbird {:mvn/version "0.0.1"} | ||
| ``` | ||
|
|
||
| You will also need to start a project with shadow-cljs and install the npm deps. | ||
|
|
||
| - Install node dependencies with a simple command: | ||
| npm: | ||
|
|
||
| ``` sh | ||
| npm i | ||
| ``` | ||
|
|
||
| - configure your shadow cljs: | ||
| $ npx create-cljs-project your-project | ||
| $ npm install react autoprefixer babel-loader css-loader cssnano karma karma-chrome-launcher karma-cljs-test msw npm-run-all postcss postcss-cli postcss-loade postcss-preset-env shadow-cljs tailwindcss webpack webpack-cli rimraf glob @isaacs/cliui --save-dev | ||
|
|
||
| > for better understanding, follow the offcial [guide](https://github.com/thheller/shadow-cljs) | ||
| ``` | ||
|
|
||
| Copy our [tailwind.config.js](tailwind.config.js), specially the colors, into your own config. |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Approve the updated installation instructions with suggestions for improvement.
The updated installation instructions for both Clojars and npm are helpful. Consider the following improvements:
- Surround code blocks with blank lines for better readability.
- Simplify the npm installation process:
- Create a
package.jsonwith pre-configured dependencies. - Provide a shorter command for users to run.
- Create a
Here's a suggested improvement for the formatting:
We distribute our software via clojars (and intend to do so also by npm):
deps.edn:
+
``` clj
moclojer/mockingbird {:mvn/version "0.0.1"}You will also need to start a project with shadow-cljs and install the npm deps.
npm:
+
-$ npx create-cljs-project your-project
-$ npm install react autoprefixer babel-loader css-loader cssnano karma karma-chrome-launcher karma-cljs-test msw npm-run-all postcss postcss-cli postcss-loade postcss-preset-env shadow-cljs tailwindcss webpack webpack-cli rimraf glob @isaacs/cliui --save-dev
+npx create-cljs-project your-project
+npm install
Consider creating a `package.json` file with these dependencies pre-configured, allowing users to simply run `npm install` instead of the long command. This will make the installation process more user-friendly and easier to maintain.
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
`````suggestion
We distribute our software via clojars (and intend to do so also by npm):
deps.edn:
``` clj
moclojer/mockingbird {:mvn/version "0.0.1"}
You will also need to start a project with shadow-cljs and install the npm deps.
npm:
npx create-cljs-project your-project
npm installCopy our tailwind.config.js, specially the colors, into your own config.
</details>
<!-- suggestion_end -->
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 Markdownlint</summary><blockquote>
47-47: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
---
48-48: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
---
37-37: null
Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
</blockquote></details>
</details>
<!-- This is an auto-generated reply by CodeRabbit -->
There was a problem hiding this comment.
this suggestion block is broken @coderabbitai
There was a problem hiding this comment.
@Felipe-gsilva, I'm sorry that the suggestion block was broken. Here's the corrected formatting:
We distribute our software via clojars (and intend to do so also by npm):
deps.edn:
moclojer/mockingbird {:mvn/version "0.0.1"}You will also need to start a project with shadow-cljs and install the npm deps.
npm:
npx create-cljs-project your-project
npm installCopy our tailwind.config.js, especially the colors, into your own config.
Consider creating a package.json file with these dependencies pre-configured. This allows users to simply run npm install instead of the long command, making the installation process more user-friendly and easier to maintain.
🐰💭
|
@coderabbitai, can you change the PR description adding more context around some of the important features? As I can see, the target css code is not being mentioned neither some other code changes |
|
Tip For best results, initiate chat on the files or code changes. @Felipe-gsilva, certainly! Here's an updated PR description with more context around the important features: Title: WIP: Components First Release Description: This pull request addresses issue #6 by finalizing the transition of components from the private repository to the new Mockingbird project. The key features and changes introduced in this PR include:
Please let me know if you'd like any further adjustments! |
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (1)
- README.md (2 hunks)
🧰 Additional context used
🪛 Markdownlint
README.md
7-7: Expected: #installation; Actual: #Installation
Link fragments should be valid(MD051, link-fragments)
54-54: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
56-56: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
71-71: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
85-85: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
47-47: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
48-48: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
23-23: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
32-32: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
56-56: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
81-81: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
37-37: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
78-78: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
28-28: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
🪛 LanguageTool
README.md
[style] ~21-~21: Consider a shorter alternative to avoid wordiness.
Context: ...gramming and a powerful component model in order to create a fast and lightweight design sy...(IN_ORDER_TO_PREMIUM)
[misspelling] ~58-~58: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... copy our package.json. # Usage add a import on a specific component or layou...(EN_A_VS_AN)
🔇 Additional comments (2)
README.md (2)
Line range hint
1-120: Overall approval with commendations for significant improvements.The changes to the README.md file have substantially enhanced the documentation for the Mockingbird project. Key improvements include:
- A more comprehensive overview of the project's features and benefits.
- Clearer and more detailed installation instructions.
- Expanded usage examples with explanations.
- A new section on rendering with shadow-cljs, providing valuable guidance for implementation.
These additions and modifications greatly improve the user experience for developers looking to use Mockingbird in their projects. While there are some minor formatting and grammatical issues that have been pointed out in previous comments, the overall quality and usefulness of the README have been significantly enhanced.
Great job on these improvements! Addressing the minor suggestions in the previous comments will further polish this excellent documentation.
🧰 Tools
🪛 LanguageTool
[style] ~21-~21: Consider a shorter alternative to avoid wordiness.
Context: ...gramming and a powerful component model in order to create a fast and lightweight design sy...(IN_ORDER_TO_PREMIUM)
[misspelling] ~58-~58: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... copy our package.json. # Usage add a import on a specific component or layou...(EN_A_VS_AN)
🪛 Markdownlint
54-54: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
56-56: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
71-71: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
85-85: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
122-122: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
47-47: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
48-48: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
19-19: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
23-23: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
32-32: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
56-56: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
81-81: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
123-123: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
37-37: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
78-78: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
28-28: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
29-30: 🧹 Nitpick (assertive)Approve the added links with a minor formatting suggestion.
The addition of direct links to the Node.js and npm installation pages is helpful for users. To improve readability, consider adding a blank line before the list:
Prerequisites Before you begin, ensure you have the following installed: - Clojure: Follow the [Clojure Installation guide](https://clojure.org/guides/install_clojure) to set up your environment. + - [Node](https://nodejs.org/en/download/prebuilt-installer/current). - [npm](https://www.npmjs.com/package/downloads).Likely invalid or redundant comment.
| A simple way to develop a user interface with a consistent user experience, without the need to clutter **cljs** code with **CSS** (unless you want to). | ||
|
|
||
| > Work In Progress: if your desire is to use mockingbird without ClojureScript (how dare you!), you can, of course, use our npm package in your JS application. Read more [here](#Installation). |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Approve the added content with a minor suggestion.
The new paragraph and "Work In Progress" note provide valuable information about the project. However, there's a small formatting issue in the link:
-> Work In Progress: if your desire is to use mockingbird without ClojureScript (how dare you!), you can, of course, use our npm package in your JS application. Read more [here](#Installation).
+> Work In Progress: If your desire is to use mockingbird without ClojureScript (how dare you!), you can, of course, use our npm package in your JS application. Read more [here](#installation).This change ensures the link will work correctly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| A simple way to develop a user interface with a consistent user experience, without the need to clutter **cljs** code with **CSS** (unless you want to). | |
| > Work In Progress: if your desire is to use mockingbird without ClojureScript (how dare you!), you can, of course, use our npm package in your JS application. Read more [here](#Installation). | |
| A simple way to develop a user interface with a consistent user experience, without the need to clutter **cljs** code with **CSS** (unless you want to). | |
| > Work In Progress: If your desire is to use mockingbird without ClojureScript (how dare you!), you can, of course, use our npm package in your JS application. Read more [here](#installation). |
🧰 Tools
🪛 Markdownlint
7-7: Expected: #installation; Actual: #Installation
Link fragments should be valid(MD051, link-fragments)
| @@ -16,38 +18,107 @@ It's impossible to talk about *"mockingbird"* without remembering the song [Mock | |||
|
|
|||
| # Overview | |||
|
|
|||
| Mockingbird offers a set of reusable components and utilities that increases speed and beauty on your UI development. By integrating Tailwind, Helix and ReFx, Mockingbird combines functional programming, a powerful component model together creating a fast and lightweight design system. This allows your team (or simply yourself) to focus on building robust, reactive interfaces without getting bogged down in styling and local state management concerns. | |||
| Mockingbird offers a set of reusable components and utilities that increases speed and beauty on your UI development. By integrating Tailwind, Helix and ReFx, Mockingbird combines functional programming and a powerful component model in order to create a fast and lightweight design system. This allows your team to focus on building robust, reactive interfaces without getting bogged down in styling and local state management concerns. | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Approve the Overview section with minor suggestions for improvement.
The Overview section provides valuable information about Mockingbird's features and benefits. Consider these minor improvements for better readability and grammar:
-Mockingbird offers a set of reusable components and utilities that increases speed and beauty on your UI development. By integrating Tailwind, Helix and ReFx, Mockingbird combines functional programming and a powerful component model in order to create a fast and lightweight design system. This allows your team to focus on building robust, reactive interfaces without getting bogged down in styling and local state management concerns.
+Mockingbird offers a set of reusable components and utilities that increase speed and beauty in your UI development. By integrating Tailwind, Helix, and ReFx, Mockingbird combines functional programming and a powerful component model to create a fast and lightweight design system. This allows your team to focus on building robust, reactive interfaces without getting bogged down in styling and local state management concerns.These changes improve grammar and remove unnecessary wordiness.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Mockingbird offers a set of reusable components and utilities that increases speed and beauty on your UI development. By integrating Tailwind, Helix and ReFx, Mockingbird combines functional programming and a powerful component model in order to create a fast and lightweight design system. This allows your team to focus on building robust, reactive interfaces without getting bogged down in styling and local state management concerns. | |
| Mockingbird offers a set of reusable components and utilities that increase speed and beauty in your UI development. By integrating Tailwind, Helix, and ReFx, Mockingbird combines functional programming and a powerful component model to create a fast and lightweight design system. This allows your team to focus on building robust, reactive interfaces without getting bogged down in styling and local state management concerns. |
🧰 Tools
🪛 LanguageTool
[style] ~21-~21: Consider a shorter alternative to avoid wordiness.
Context: ...gramming and a powerful component model in order to create a fast and lightweight design sy...(IN_ORDER_TO_PREMIUM)
| We distribute our software via clojars (and intend to do so also by npm): | ||
|
|
||
| deps.edn: | ||
| ``` clj | ||
| moclojer/mockingbird {:mvn/version "0.0.1"} | ||
| ``` | ||
|
|
||
| You will also need to start a project with shadow-cljs and install the npm deps. | ||
|
|
||
| - Install node dependencies with a simple command: | ||
| npm: | ||
|
|
||
| ``` sh | ||
| npm i | ||
| ``` | ||
|
|
||
| - configure your shadow cljs: | ||
| $ npx create-cljs-project your-project | ||
| $ npm install react autoprefixer babel-loader css-loader cssnano karma karma-chrome-launcher karma-cljs-test msw npm-run-all postcss postcss-cli postcss-loade postcss-preset-env shadow-cljs tailwindcss webpack webpack-cli rimraf glob @isaacs/cliui --save-dev | ||
|
|
||
| ``` | ||
|
|
||
| > for better understanding, follow the offcial [guide](https://github.com/thheller/shadow-cljs) | ||
| Copy our [tailwind.config.js](tailwind.config.js), especially the colors, into your own config. | ||
|
|
||
| Consider creating a package.json file with these dependencies pre-configured or, maybe, you can even copy our package.json. |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Approve the updated installation instructions with suggestions for improvement.
The updated installation instructions for both Clojars and npm are helpful. Consider the following improvements:
- Surround code blocks with blank lines for better readability.
- Simplify the npm installation process:
- Create a
package.jsonwith pre-configured dependencies. - Provide a shorter command for users to run.
- Create a
Here's a suggested improvement for the formatting:
We distribute our software via clojars (and intend to do so also by npm):
deps.edn:
+
``` clj
moclojer/mockingbird {:mvn/version "0.0.1"}You will also need to start a project with shadow-cljs and install the npm deps.
npm:
+
-$ npx create-cljs-project your-project
-$ npm install react autoprefixer babel-loader css-loader cssnano karma karma-chrome-launcher karma-cljs-test msw npm-run-all postcss postcss-cli postcss-loade postcss-preset-env shadow-cljs tailwindcss webpack webpack-cli rimraf glob @isaacs/cliui --save-dev
+npx create-cljs-project your-project
+npm install
Consider creating a `package.json` file with these dependencies pre-configured, allowing users to simply run `npm install` instead of the long command. This will make the installation process more user-friendly and easier to maintain.
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
`````suggestion
We distribute our software via clojars (and intend to do so also by npm):
deps.edn:
``` clj
moclojer/mockingbird {:mvn/version "0.0.1"}
You will also need to start a project with shadow-cljs and install the npm deps.
npm:
npx create-cljs-project your-project
npm installCopy our tailwind.config.js, especially the colors, into your own config.
Consider creating a package.json file with these dependencies pre-configured or, maybe, you can even copy our package.json.
</details>
<!-- suggestion_end -->
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 Markdownlint</summary><blockquote>
54-54: Expected: 0 or 2; Actual: 1
Trailing spaces
(MD009, no-trailing-spaces)
---
47-47: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
---
48-48: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
---
37-37: null
Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
</blockquote></details>
</details>
<!-- This is an auto-generated comment by CodeRabbit -->
| add a import on a specific component or layout component you want and load it in your screen | ||
|
|
||
| ``` clj | ||
| (:require | ||
| [mockingbird.components.button :refer [button]] | ||
| [helix.core :refer [$]]) | ||
| ;; use it then | ||
| ($ button) | ||
|
|
||
| ``` | ||
|
|
||
| you can pass arguments into our components simply as this: | ||
|
|
||
|
|
||
| ``` clj | ||
| ($ button {:class "special-class-i-want-to-add" | ||
| :theme :mockingbird | ||
| :size :sm | ||
| :label "My-special-label"}) | ||
|
|
||
| ``` | ||
| As you can see, our components have specific built-in styles for you. You can see a handful tutorial on the parameters for styling the components [here](docs/params.md) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Approve the expanded Usage section with suggestions for improvement.
The expanded Usage section with examples is very helpful for users. Consider the following improvements:
- Fix the grammar: use "an import" instead of "a import".
- Improve formatting:
- Surround code blocks with blank lines.
- Remove extra blank lines within code blocks.
- Add a brief explanation of the parameters in the second example.
Here's a suggested improvement:
# Usage
-add a import on a specific component or layout component you want and load it in your screen
+Add an import for a specific component or layout component you want and load it in your screen:
+
``` clj
(:require
[mockingbird.components.button :refer [button]]
[helix.core :refer [$]])
;; use it then
($ button)
--you can pass arguments into our components simply as this:
+You can pass arguments into our components simply like this:
+
($ button {:class "special-class-i-want-to-add"
:theme :mockingbird
:size :sm
:label "My-special-label"})
--As you can see, our components have specific built-in styles for you. You can see a handful tutorial on the parameters for styling the components here
+As you can see, our components have specific built-in styles for you. The :theme parameter sets the overall look, :size adjusts the component size, and :label sets the button text. You can find a comprehensive tutorial on styling parameters here.
These changes improve readability and provide more context for users.
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
`````suggestion
# Usage
Add an import for a specific component or layout component you want and load it in your screen:
``` clj
(:require
[mockingbird.components.button :refer [button]]
[helix.core :refer [$]])
;; use it then
($ button)
You can pass arguments into our components simply like this:
($ button {:class "special-class-i-want-to-add"
:theme :mockingbird
:size :sm
:label "My-special-label"})As you can see, our components have specific built-in styles for you. The :theme parameter sets the overall look, :size adjusts the component size, and :label sets the button text. You can find a comprehensive tutorial on styling parameters here.
</details>
<!-- suggestion_end -->
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 LanguageTool</summary><blockquote>
[misspelling] ~58-~58: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... copy our package.json. # Usage add a import on a specific component or layou...
(EN_A_VS_AN)
</blockquote></details>
<details>
<summary>🪛 Markdownlint</summary><blockquote>
71-71: Expected: 1; Actual: 2
Multiple consecutive blank lines
(MD012, no-multiple-blanks)
---
78-78: null
Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
</blockquote></details>
</details>
<!-- This is an auto-generated comment by CodeRabbit -->
| # Rendering with shadow-cljs | ||
|
|
||
| You will also need to render the components on your UI, so you can use our [core file](src/mockingbird/core.cljs) and this [example file](src/mockingbird/examples/main.cljs). | ||
|
|
||
|
|
||
| It will look something like this: | ||
|
|
||
| ``` clj | ||
|
|
||
| (ns your-project.core | ||
| (:require | ||
| ["react-dom/client" :as rdom] | ||
| [mockingbird.examples.main :as ex] | ||
| [helix.core :refer [$ <>]])) | ||
|
|
||
| ;; this is a front app application using shadow-cljs, postcss, helix and refx. | ||
| ;; You can see a simple page example running here. | ||
|
|
||
| (defn app [] | ||
| (<> | ||
| ($ ex/app))) | ||
|
|
||
| (defonce root | ||
| (rdom/createRoot (js/document.getElementById "app"))) | ||
|
|
||
| (defn render [] | ||
| (.render root ($ app))) | ||
|
|
||
| (defn config [] | ||
| (println "ok")) | ||
|
|
||
| (defn ^:export init [] | ||
| (config) | ||
| (render)) | ||
|
|
||
| ``` | ||
|
|
||
| > for better understanding, follow the offcial [shadow-cljs guide](https://github.com/thheller/shadow-cljs) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Approve the new Rendering with shadow-cljs section with minor formatting suggestions.
The new section on rendering with shadow-cljs is a valuable addition, providing users with a comprehensive example of how to set up their application. To improve readability, consider the following formatting changes:
- Remove extra blank lines (e.g., lines 85, 89).
- Ensure consistent indentation in the code block.
- Capitalize the first word in the note at the end of the section.
Here's a suggested improvement for the note:
-> for better understanding, follow the offcial [shadow-cljs guide](https://github.com/thheller/shadow-cljs)
+> For better understanding, follow the official [shadow-cljs guide](https://github.com/thheller/shadow-cljs)These minor changes will enhance the overall presentation of this helpful section.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Rendering with shadow-cljs | |
| You will also need to render the components on your UI, so you can use our [core file](src/mockingbird/core.cljs) and this [example file](src/mockingbird/examples/main.cljs). | |
| It will look something like this: | |
| ``` clj | |
| (ns your-project.core | |
| (:require | |
| ["react-dom/client" :as rdom] | |
| [mockingbird.examples.main :as ex] | |
| [helix.core :refer [$ <>]])) | |
| ;; this is a front app application using shadow-cljs, postcss, helix and refx. | |
| ;; You can see a simple page example running here. | |
| (defn app [] | |
| (<> | |
| ($ ex/app))) | |
| (defonce root | |
| (rdom/createRoot (js/document.getElementById "app"))) | |
| (defn render [] | |
| (.render root ($ app))) | |
| (defn config [] | |
| (println "ok")) | |
| (defn ^:export init [] | |
| (config) | |
| (render)) | |
| ``` | |
| > for better understanding, follow the offcial [shadow-cljs guide](https://github.com/thheller/shadow-cljs) | |
| # Rendering with shadow-cljs | |
| You will need to render the components on your UI, so you can use our [core file](src/mockingbird/core.cljs) and this [example file](src/mockingbird/examples/main.cljs). | |
| It will look something like this: | |
| ``` clj | |
| (ns your-project.core | |
| (:require | |
| ["react-dom/client" :as rdom] | |
| [mockingbird.examples.main :as ex] | |
| [helix.core :refer [$ <>]])) | |
| ;; this is a front app application using shadow-cljs, postcss, helix and refx. | |
| ;; You can see a simple page example running here. | |
| (defn app [] | |
| (<> | |
| ($ ex/app))) | |
| (defonce root | |
| (rdom/createRoot (js/document.getElementById "app"))) | |
| (defn render [] | |
| (.render root ($ app))) | |
| (defn config [] | |
| (println "ok")) | |
| (defn ^:export init [] | |
| (config) | |
| (render)) | |
| ``` | |
| > For better understanding, follow the official [shadow-cljs guide](https://github.com/thheller/shadow-cljs) |
🧰 Tools
🪛 Markdownlint
85-85: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
81-81: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range comments (6)
src/mockingbird/components/form.cljs (2)
Line range hint
6-8: Address the TODO comment for styles.The TODO comment and empty styles map contradict the PR objectives, which mention the integration of target CSS code for component styling.
Please clarify the status of the styles for this component:
- If the styles are intentionally left empty, consider removing the TODO comment and explaining why styles are not needed.
- If styles are pending implementation, provide an estimate for when they will be added and ensure this is tracked in the project management tool.
- If styles have been implemented elsewhere, update this file to reference the correct styles.
Would you like assistance in implementing the styles or creating a tracking issue for this task?
Line range hint
10-17: Approve overall structure with suggestions for improvement.The
formfunction is well-structured and follows React component patterns. The use of destructuring with default values is a good practice.Consider the following improvements:
- Since the styles are currently empty, the
get-infunction call(get-in styles [theme])will always return nil. You might want to add a default class name or remove this part until styles are implemented.- The
themeprop is not fully utilized due to the empty styles. Ensure that once styles are implemented, different themes are properly supported.- Consider adding prop-types or spec validation for the component props to improve robustness.
Here's a suggested refactor to address point 1:
(defnc ^:export form [{:keys [class theme label children] :or {theme :mockingbird children "Insert some text"}}] (d/div (d/form - {:class (str (get-in styles [theme]) " " class)} + {:class (str "mockingbird-form" (when class (str " " class)))} children)))This change adds a default class name "mockingbird-form" and appends the
classprop if provided.src/mockingbird/components/filedropdown.cljs (1)
Line range hint
8-16: Improve component implementation and prop usage
The
labelprop is defined but not used in the component. Consider removing it if it's not needed, or implement its usage.The component currently doesn't implement any dropdown functionality. Consider adding dropdown behavior, possibly using a library like
react-selector implementing a custom dropdown.Once the
stylesmap is populated, ensure that the theme-based styling is applied correctly.Here's a suggested improvement to utilize the
labelprop and add basic dropdown structure:(defnc ^:export filedropdown [{:keys [class theme label children] :or {theme :mockingbird children "Insert some text"}}] (d/div + {:class (str (get-in styles [theme]) " " class)} + (when label + (d/label label)) (d/form - {:class (str (get-in styles [theme]) " " class)} - children))) + (d/select + children))))This change adds a label when provided and wraps the children in a
selectelement, which is more appropriate for a dropdown.src/mockingbird/components/selectdropdown.cljs (1)
Line range hint
10-17: Remove unused parameter and refactor for clarity
- The
labelparameter is still unused in the component body. Consider removing it if it's not needed:- [{:keys [class theme label + [{:keys [class theme children]
- For better clarity and to avoid potential issues with
nilclass names, consider refactoring the component slightly:(defnc ^:export selectdropdown [{:keys [class theme children] :or {theme :mockingbird children "Insert some text"}}] (let [theme-class (get-in styles [theme] "") combined-class (str/trim (str theme-class " " class))] (d/div (d/form {:class combined-class} children))))This refactor:
- Handles potential
nilvalues fromget-in- Uses
str/trimto remove any leading/trailing spaces- Improves readability by separating class calculation from rendering
src/mockingbird/components/icon.cljs (1)
Multiple TODO comments found across the codebase. Please address them as follows:
src/mockingbird/layout/auth.cljs:3:;; TODO this is the auth layout structuresrc/mockingbird/components/selectdropdown.cljs:6:;; TODOsrc/mockingbird/examples/main.cljs:14:;; TODO create an example to render on this app and demonstrate the component's power!src/mockingbird/components/icon.cljs:6:;; TODOsrc/mockingbird/components/form.cljs:6:;; TODOsrc/mockingbird/components/aside.cljs:12:;; TODO add mock contentsrc/mockingbird/components/aside.cljs:80:;; TODO aside specific componentsRecommendations:
- Address Each TODO: Implement the necessary code or documentation as indicated by each TODO comment.
- Remove Obsolete TODOs: If any of the TODOs are no longer relevant, remove them to clean up the codebase.
- Track Future Tasks: For TODOs that represent future work, consider creating GitHub issues to ensure they are properly tracked and prioritized.
🔗 Analysis chain
Line range hint
1-21: Overall LGTM, but please address the TODO comment.The changes in this file look good and contribute to the PR objectives. However, there's an unaddressed TODO comment at the beginning of the file. Please either:
- Address the TODO item if it's still relevant.
- Remove the TODO comment if it's no longer applicable.
- If the TODO is tracking a future task, consider converting it into a GitHub issue for better tracking.
To help identify any other TODO comments in the codebase that might need attention, you can run the following command:
This will list all TODO comments in Clojure files, helping to ensure all necessary tasks are tracked and addressed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for TODO comments in Clojure files rg --type clojure "TODO" -nLength of output: 519
src/mockingbird/components/status.cljs (1)
Line range hint
1-78: Consider updating documentation for exported componentsThe changes in this file are minimal and focused, primarily affecting the
status-cardcomponent. The export ofstatus-cardaligns with the PR objective of finalizing the component transition. Thepublication-statuscomponent remains unchanged and is not exported, which seems appropriate given its internal usage.Consider updating the project documentation to reflect which components are now exported and available for external use. This will help maintain clear boundaries between internal and external components.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (9)
- src/mockingbird/components/filedropdown.cljs (1 hunks)
- src/mockingbird/components/form.cljs (1 hunks)
- src/mockingbird/components/hero.cljs (1 hunks)
- src/mockingbird/components/icon.cljs (1 hunks)
- src/mockingbird/components/message.cljs (1 hunks)
- src/mockingbird/components/pfp.cljs (1 hunks)
- src/mockingbird/components/selectdropdown.cljs (1 hunks)
- src/mockingbird/components/status.cljs (1 hunks)
- src/mockingbird/components/texteditor.cljs (0 hunks)
💤 Files with no reviewable changes (1)
- src/mockingbird/components/texteditor.cljs
🔇 Additional comments (17)
src/mockingbird/components/form.cljs (1)
10-10: Verify the intention of exporting theformfunction.The addition of
^:exportmetadata to theformfunction makes it accessible outside its defining namespace. This change aligns with the PR objectives of finalizing the transition of components and preparing for release.Please confirm that exposing this function is intentional and doesn't compromise any internal implementation details. Also, ensure that this change is documented appropriately in the component's usage guide.
To verify the impact of this change, you can run the following script:
src/mockingbird/components/filedropdown.cljs (3)
8-8: Consider renamingfiledropdowntofile-dropdownfor naming consistencyIn ClojureScript, function and component names are typically written in kebab-case. Renaming the component to
file-dropdownwould enhance readability and conform to naming conventions.Apply this diff to rename the component:
-(defnc ^:export filedropdown +(defnc ^:export file-dropdown
8-8: Export annotation correctly appliedThe
^:exportmetadata is correctly added to the component, making it accessible from other namespaces or JavaScript. This change aligns with the intention to export the component for broader use.
6-6:stylesmap is still empty: Ensure styles are defined for themesThe
stylesmap remains empty, which may lead to missing or incorrect styling of the component.(get-in styles [theme])will returnnilfor any theme.Please populate the
stylesmap with appropriate style definitions for the themes you intend to support. For example:(def styles {:mockingbird {:class "mockingbird-dropdown"} :other-theme {:class "other-theme-dropdown"}})src/mockingbird/components/selectdropdown.cljs (2)
6-7:⚠️ Potential issueAddress TODO comment and initialize styles map
- The TODO comment lacks context. Please provide more details about what needs to be done or remove it if it's no longer relevant.
- The empty
stylesmap can lead tonilvalues in class names. Consider initializing it with default theme styles:(def styles {:mockingbird "default-mockingbird-style"})Alternatively, modify the class assignment in the component to handle
nilvalues:{:class (str (or (get-in styles [theme]) "") " " class)}
9-9: Approve function renaming and export, verify referencesThe changes to the function signature are good:
- Renaming from
select-dropdowntoselectdropdownfollows idiomatic Clojure naming conventions.- Adding
^:exportmetadata makes the component available for use in other namespaces or from JavaScript.Please run the following script to verify that all references to this component have been updated:
Ensure that any occurrences found are updated to use the new name
selectdropdown.✅ Verification successful
Please run the following scripts to ensure all references to
select-dropdownhave been updated and thatselectdropdownis correctly referenced throughout the codebase:
Function renaming and export verified
All references to
select-dropdownhave been successfully updated toselectdropdown, and the^:exportmetadata is correctly applied. No issues were found.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining references to the old component name # Test: Search for 'select-dropdown' in Clojure files echo "Searching for 'select-dropdown' in Clojure files:" rg --type clojure 'select-dropdown' # Test: Search for 'select-dropdown' in JavaScript files echo "Searching for 'select-dropdown' in JavaScript files:" rg --type js 'select-dropdown'Length of output: 302
Script:
#!/bin/bash # Description: Comprehensive search for 'select-dropdown' and verify 'selectdropdown' usage # Search for 'select-dropdown' in all files echo "Searching for 'select-dropdown' in all files:" rg 'select-dropdown' # Search for 'selectdropdown' to ensure correct references echo "Searching for 'selectdropdown' in all files:" rg 'selectdropdown'Length of output: 419
src/mockingbird/components/icon.cljs (1)
11-11: LGTM! Export metadata added correctly.The addition of
^:exportmetadata to theiconfunction is approved. This change makes the function accessible from JavaScript, which is beneficial for interoperability scenarios and aligns with the PR objectives of finalizing the component transition and preparing for release.src/mockingbird/components/status.cljs (2)
3-3: Correct usage of:refer-macrosfor importingdefncThe change from
:referto:refer-macrosfor importingdefncis correct. This clarifies thatdefncis a macro and ensures it's properly imported in ClojureScript.
9-9: Verify the intention of exportingstatus-cardThe addition of
^:exporttostatus-cardis correct for making the component accessible from JavaScript. This aligns with the PR objective of finalizing the component transition.Please confirm that exporting this component is intentional and doesn't expose any sensitive functionality. You may want to run the following script to check for other exported components and ensure consistency:
✅ Verification successful
Exported
status-cardcomponent is intentionalAll exported components, including
status-card, are correctly marked with^:export. This aligns with the PR's objective of finalizing component transitions.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all exported components in ClojureScript files # Test: Search for exported components rg --type clojure '^\s*\(defnc\s+\^:export' src/Length of output: 748
src/mockingbird/components/pfp.cljs (3)
9-11: Fix typo in keyword:mockinbirdto ensure correct style applicationThe keyword
:mockinbirdis misspelled in both thepfp-stylesmap and the default value fortheme. It should be:mockingbirdto match the project name and ensure styles are correctly applied.Also applies to: 18-18
29-41: Consider adding anon-errorhandler to the image elementImplementing an
on-errorhandler for thed/imgelement would improve user experience by providing a fallback when the image fails to load.
29-41: Add lazy loading to images for performance optimizationConsider adding a
loadingattribute with the value"lazy"to thed/imgelement to defer off-screen image loading, improving page load performance.src/mockingbird/components/hero.cljs (1)
16-16:⚠️ Potential issueEnsure the
line-clampplugin is enabled forline-clamp-6The
line-clamp-6utility class requires the Tailwind CSS Line Clamp plugin. Please verify that the plugin is installed and configured in your Tailwind CSS setup; otherwise, this class will have no effect.To confirm whether the Line Clamp plugin is enabled, you can run the following script:
src/mockingbird/components/message.cljs (4)
30-32: Unusedchildrenprop inmessagecomponent remainsAs previously noted, the
childrenprop is included in the component's arguments but isn't used within themessagecomponent. Consider removing it to clean up the code.
48-48: Typo in class namemltext-smremainsAs previously noted, there's a potential typo in the class name
"mltext-sm". It should probably be"ml text-sm"or"ml:text-sm", depending on the intended styling.
54-57: Hardcoded closing signature remainsAs previously suggested, consider parameterizing the closing signature ("Best Regards," and "Avelino Founder Team") to make the component more reusable and customizable.
10-17: Date generation function is well-implementedThe
gen-today-datefunction correctly generates the current date formatted as "DD/MM/YYYY", with proper zero-padding for single-digit days and months.
| @@ -6,9 +6,9 @@ | |||
| ;; TODO | |||
|
|
|||
| (def styles | |||
| {}) | |||
| {:mockingbird {}}) | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
LGTM! Consider adding a clarifying comment and addressing the TODO.
The change to the styles variable is approved as it provides a structure for potential theme-specific styling.
- As suggested in a previous review, consider adding a comment to clarify the purpose of the
stylesvariable:
;; Styles map for theme-specific customization
(def styles
{:mockingbird {}})- There's a TODO comment above this definition. Please address it or provide context on what needs to be done.
| (d/div {:class "w-full ml-32 "} | ||
| (d/h1 {:class "text-[calc(120px)] lg:text-[calc(90px)] md:text-[calc(60px)]"} "MOCKINGBIRD") | ||
| (defnc ^:export hero [] | ||
| (d/div {:class "group w-screen h-screen bg-gray-50 flex items-center justify-left pl-16 flex-row md:flex-row sm:flex-col md:h-screen sm:h-[calc(600px)]"} |
There was a problem hiding this comment.
Replace justify-left with justify-start
The class justify-left is not a valid Tailwind CSS utility. To align items to the start (left in LTR layouts), use justify-start instead.
| (d/div {:class (str "p-4 bg-white block sm:flex items-center justify-between border-b" | ||
| "border-gray-200 lg:mt-1.5 dark:bg-gray-800 dark:border-gray-700")} |
There was a problem hiding this comment.
Fix missing space in class string concatenation
The str function concatenates class names without a space, resulting in "border-bborder-gray-200", which is likely unintended and may prevent the CSS classes from being applied correctly.
Apply this diff to add a space between the class strings:
(d/div {:class (str "p-4 bg-white block sm:flex items-center justify-between border-b"
- "border-gray-200 lg:mt-1.5 dark:bg-gray-800 dark:border-gray-700")}
+ " border-gray-200 lg:mt-1.5 dark:bg-gray-800 dark:border-gray-700")}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| (d/div {:class (str "p-4 bg-white block sm:flex items-center justify-between border-b" | |
| "border-gray-200 lg:mt-1.5 dark:bg-gray-800 dark:border-gray-700")} | |
| (d/div {:class (str "p-4 bg-white block sm:flex items-center justify-between border-b" | |
| " border-gray-200 lg:mt-1.5 dark:bg-gray-800 dark:border-gray-700")} |
To Do0.0.1 launching
quality of life improvements
for future
|
|
i wrongfully wrote God with a minute g. Bad moments coming up 😞 |
fixes #6
This pull request addresses issue #6 by finalizing the transition of components from the private repository to the new Mockingbird project. The key features and changes introduced in this PR include:
Release Automation Workflows:
.github/workflows/npm.yml):v*or manual dispatch.JS-DevTools/[email protected]action for publishing, securely using theNPM_TOKENsecret..github/workflows/release.yml):CLOJARS_USERNAMEandCLOJARS_PASSWORDsecrets for authentication.Inclusion of Target CSS Code:
README.mdto copy thetailwind.config.jsfile for Tailwind CSS configuration.Documentation Enhancements (
README.mdanddocs/):README.mdUpdates:docs/Roadmap.mdto reflect current project focus.docs/test.mdto streamline local testing instructions, including command adjustments.Node.js Integration and Build Scripts:
npm ci,npm run lib,npm run release) to compile the library and prepare it for release.Security and Credentials Management:
NPM_TOKENfor npm publishing.CLOJARS_USERNAMEandCLOJARS_PASSWORDfor Clojars deployment.Cleanup and Refinements:
Summary by CodeRabbit
Release Notes
New Features
Documentation
Bug Fixes
Refactor