-
Notifications
You must be signed in to change notification settings - Fork 0
chore(dev): align dev compose ports + hooks target #33
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
zensgit
commented
Sep 24, 2025
- Align jive-api/docker-compose.dev.yml ports with manager: DB 5433, Redis 6380, Adminer 9080\n- Add Makefile hooks target to enable pre-commit (api-lint)\n\nValidation:\n- ./jive-manager.sh start all-dev should now map to the same ports as compose.\n- Adminer reachable at http://localhost:9080\n\nCI: Expect no behavior changes; just dev environment consistency.
…80); add Makefile hooks target
Summary of ChangesHello @zensgit, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request streamlines the development environment by standardizing port configurations for key services in Docker Compose and enhancing code quality practices. It ensures that development ports for PostgreSQL, Redis, and Adminer are consistent with the jive-manager setup, reducing configuration discrepancies. Additionally, it introduces a convenient Makefile target to enable Git pre-commit hooks, facilitating automated linting and maintaining code standards. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Pull Request Overview
This PR aligns development environment port configurations between the API's Docker Compose setup and the manager script, and adds a new git hooks target to enable pre-commit linting.
- Port standardization across development tools for consistency
- Addition of git hooks configuration for automated code quality checks
- Minor CI workflow enhancement to include docs-only optimization tracking
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| jive-api/docker-compose.dev.yml | Updates port mappings to align with manager script defaults (DB 5433, Redis 6380, Adminer 9080) |
| Makefile | Adds new hooks target to configure git pre-commit hooks for api-lint |
| .pids/api.mode | Creates new file to track API development mode state |
| .github/workflows/ci.yml | Enhances CI summary with docs-only optimization tracking |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
.pids/api.mode
Outdated
| @@ -0,0 +1 @@ | |||
| dev | |||
Copilot
AI
Sep 24, 2025
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.
This new file appears to be tracking API mode state but lacks documentation. Consider adding a comment or README explaining its purpose and how it's used in the development workflow.
Copilot uses AI. Check for mistakes.
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.
Code Review
This pull request aligns development environment ports for consistency and adds a Makefile target for git hooks. The port alignment changes in docker-compose.dev.yml are correct and well-justified. However, I've identified two issues: a duplicated hooks target in the Makefile which should be consolidated, and a new runtime state file (.pids/api.mode) being added to version control, which should be git-ignored instead. Addressing these points will improve the maintainability and developer experience of the repository.
| .PHONY: hooks | ||
| hooks: | ||
| @git config core.hooksPath .githooks | ||
| @echo "Git hooks enabled (pre-commit will run make api-lint)" |
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.
This change introduces a hooks target that duplicates an existing one on lines 148-150. Having duplicate targets in a Makefile can lead to unpredictable behavior and makes the file harder to maintain.
Please remove this duplicated block. You can enhance the existing target by adding .PHONY: hooks above it for correctness, like so:
.PHONY: hooks
hooks:
@git config core.hooksPath .githooks
@echo "✅ Git hooks enabled (pre-commit runs make api-lint)"
.pids/api.mode
Outdated
| @@ -0,0 +1 @@ | |||
| dev | |||
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.
The .pids directory seems intended for runtime-generated files, such as process IDs and service modes. These files are specific to a developer's local environment and should not be tracked in version control.
Committing this file can lead to unnecessary merge conflicts and noise in the repository history. It's recommended to add the .pids/ directory to your project's .gitignore file.
…ss while keeping required checks
|
Ready to merge once approved. Summary:\n- Align dev compose ports with manager (DB 5433, Redis 6380, Adminer 9080)\n- Add Makefile hooks target for pre-commit (api-lint)\n- CI: Flutter job short-circuits when no Flutter changes\n\nLocal verify after merge:\n- ./jive-manager.sh stop && ./jive-manager.sh start all-dev\n- DB: psql postgresql://postgres:postgres@localhost:5433/jive_money -c 'select 1'\n- Adminer: http://localhost:9080\n- API: curl -s http://localhost:8012/health |
…ompare on flutter_changed
…vent in concurrency key
- Add conditional execution to field-compare job - Only run when flutter_changed == 'true' - Fixes artifact not found error when Flutter tests are skipped