-
Notifications
You must be signed in to change notification settings - Fork 24
ci: add poe install
definition to save some keystrokes
#154
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughThe modifications in the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
pyproject.toml (4)
127-128
: Consider adding a lightweight install variant?The
install
task with--all-extras
is great for full development setup, but what do you think about adding a lightweight variant without extras for faster installation when working on core features? Something like:# Installation install = { shell = "poetry install --all-extras" } +install-core = { shell = "poetry install", help = "Install only core dependencies without extras" }
wdyt? 🤔
Line range hint
133-156
: Add documentation for ignore_fail behavior?The format and lint tasks look great! Would it be helpful to add a comment explaining why
ignore_fail = "return_non_zero"
is used in these tasks? This could help other developers understand the behavior, something like:# Format check tasks +# ignore_fail is used to continue execution even if one formatter fails, allowing all format issues to be reported _format-check-ruff = {cmd = "ruff format --check .", help = "Check formatting with Ruff."}
What are your thoughts on this? 🤔
Line range hint
169-171
: Consider enhancing coverage reporting options?The test tasks look solid! Would it be valuable to add HTML coverage reports for better visualization? Maybe something like:
-unit-test-with-cov = {cmd = "pytest -s unit_tests --cov=airbyte_cdk --cov-report=term --cov-config ./pyproject.toml", help = "Run unit tests and create a coverage report."} +unit-test-with-cov = {cmd = "pytest -s unit_tests --cov=airbyte_cdk --cov-report=term --cov-report=html --cov-config ./pyproject.toml", help = "Run unit tests and create coverage reports (terminal + HTML)."}This would generate a browsable coverage report. What do you think? 📊
Line range hint
175-176
: Address the TODO comment about mypy checkThere's a TODO about finding a version of the modified mypy check that works both locally and in CI. Would you like help investigating and implementing this? I can help create a solution that works consistently across environments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
pyproject.toml
(1 hunks)
🔇 Additional comments (1)
pyproject.toml (1)
129-132
: Verify the manifest generation script
Let's ensure the shell script exists and has proper permissions.
✅ Verification successful
All good with the manifest generation script! 🎯
The script setup looks solid - both the shell script and its Python companion exist and are properly configured. The shell script handles the Docker environment setup for consistent codegen, and the Python script takes care of the actual manifest generation with proper version pinning. Everything's in place and executable!
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the script exists and is executable
if [ -x "bin/generate-component-manifest-dagger.sh" ]; then
echo "✅ Script exists and is executable"
else
echo "❌ Script not found or not executable"
# Check if it exists but isn't executable
if [ -f "bin/generate-component-manifest-dagger.sh" ]; then
echo "Found script but it's not executable"
fi
fi
Length of output: 131
Script:
#!/bin/bash
# Let's check the content of the script to understand what it does
cat bin/generate-component-manifest-dagger.sh
Length of output: 461
Script:
#!/bin/bash
# Let's verify if the Python script it calls exists and check its content
if [ -f "bin/generate_component_manifest_files.py" ]; then
echo "✅ Python script exists"
echo "Content of the Python script:"
cat bin/generate_component_manifest_files.py
else
echo "❌ Python script not found"
fi
Length of output: 3470
Summary by CodeRabbit
New Features
Improvements