fix(local): fix missing doc and target about plugin compilation #422
Conversation
WalkthroughThis change introduces a new shell script for generating a Go plugin list file, updates build and development workflows to use this script, and enhances the build system to optionally save the generated file locally. The documentation is updated to reflect the new workflow, and the pre-commit process is extended to include plugin compilation. The Earthfile's Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant Justfile
participant compile-plugin.sh
participant Earthfile
Developer->>Justfile: Run pre-commit or compile-plugins
Justfile->>compile-plugin.sh: Execute with list.go and plugin dir
compile-plugin.sh->>compile-plugin.sh: Generate list.go with imports
Justfile->>Earthfile: Optionally trigger compile-plugins with local_save
Earthfile->>compile-plugin.sh: Execute script as part of build
Earthfile->>Earthfile: Conditionally save list.go locally if local_save=true
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
d2951a8 to
c19200f
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
tools/compile-plugins/compile-plugin.sh (1)
10-12: Command substitution can break with spaces in directory names.The loop may have issues with directory names containing spaces. Also,
ls -d */isn't a reliable way to list directories as it doesn't handle hidden directories and can be affected by shell globbing.-for c in $(ls -d */ | sed 's#/##'); do - printf " _ \"github.com/formancehq/payments/internal/connectors/plugins/public/$c\"\n" >> $1 -done +find . -mindepth 1 -maxdepth 1 -type d | while read -r dir; do + c=$(basename "$dir") + printf " _ \"github.com/formancehq/payments/internal/connectors/plugins/public/%s\"\n" "$c" >> "$1" +doneCONTRIBUTING.md (2)
745-745: Fix Markdown linting issue with command documentation.The line violates the Markdown linting rule MD014 which recommends not using dollar signs before commands without showing output.
-$ earthly -P +compile-plugins --local_save=true +```bash +earthly -P +compile-plugins --local_save=true +```🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
745-745: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
744-747: Document the plugin compilation step purpose.The new command is added without any explanation about what it does or why it's necessary before running the service.
+```bash +# First compile the Go plugins and save locally +earthly -P +compile-plugins --local_save=true -$ earthly -P +compile-plugins --local_save=true $ docker compose up +``` + +The first command generates a `list.go` file with all available plugins, which is necessary for the Connectivity Service to discover and load the plugins.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
745-745: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
746-746: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
Earthfile (1)
23-23: Consider proper error handling for the compile-plugin.sh script.The script execution doesn't verify if the script runs successfully. If it fails, the build might continue with an invalid or missing list.go file.
- RUN ./src/tools/compile-plugins/compile-plugin.sh list.go /src/internal/connectors/plugins/public + RUN set -e && \ + if [ ! -f ./src/tools/compile-plugins/compile-plugin.sh ]; then \ + echo "Error: compile-plugin.sh script not found!" && \ + exit 1; \ + fi && \ + ./src/tools/compile-plugins/compile-plugin.sh list.go /src/internal/connectors/plugins/public
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CONTRIBUTING.md(1 hunks)Earthfile(1 hunks)Justfile(2 hunks)tools/compile-plugins/compile-plugin.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
tools/compile-plugins/compile-plugin.sh
[warning] 7-7: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 14-14: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🪛 markdownlint-cli2 (0.17.2)
CONTRIBUTING.md
745-745: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Tests
🔇 Additional comments (2)
Justfile (1)
6-6: Good integration of compile-plugins into the pre-commit workflow.The change correctly integrates the new compile-plugins step into the pre-commit workflow.
Earthfile (1)
20-27: Good implementation of conditional local file saving.The changes to the compile-plugins target improve the build workflow by allowing optional local saving with a clean conditional statement.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #422 +/- ##
=======================================
Coverage 60.93% 60.93%
=======================================
Files 566 566
Lines 28843 28843
=======================================
Hits 17576 17576
Misses 10317 10317
Partials 950 950 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
c19200f to
02b324f
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
CONTRIBUTING.md (1)
745-745: Remove leading$prompt in shell code block
Per markdownlint rule MD014, avoid including prompt characters without showing command output. Consider removing the$or adding expected output.-$ earthly -P +compile-plugins --local_save=true +earthly -P +compile-plugins --local_save=true🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
745-745: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CONTRIBUTING.md(1 hunks)Earthfile(1 hunks)Justfile(2 hunks)tools/compile-plugins/compile-plugin.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- Justfile
- tools/compile-plugins/compile-plugin.sh
- Earthfile
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
CONTRIBUTING.md
745-745: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Tests
No description provided.