add precommit and fix postgres job #418
Conversation
WalkthroughA new GitHub Actions workflow named Changes
Possibly related PRs
Poem
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 (
|
klinch0
force-pushed
the
bugfix/fix_postgres_job
branch
from
910a026 to
79f587e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (10)
.pre-commit-config.yaml (1)
12-16: LGTM! Consider documenting the disabled markdownlint rules.The markdownlint configuration looks good. Using the
--fixargument is helpful for automatically resolving issues.Suggestion: Consider adding a comment explaining why rules MD013 (line length) and MD041 (first line should be a top-level heading) are disabled. This will help future maintainers understand the rationale behind these choices.
Example:
- id: markdownlint args: - --fix - --disable - MD013 # Line length - MD041 # First line in file should be a top-level heading - -- # MD013 and MD041 are disabled because....github/workflows/pre-commit.yml (2)
5-7: Consider specifying an Ubuntu version for consistency.While using
ubuntu-latestis generally good, specifying a version (e.g.,ubuntu-22.04) can provide more consistency across runs and prevent unexpected issues if the latest version changes.You could update the line as follows:
- runs-on: ubuntu-latest + runs-on: ubuntu-22.04
17-18: LGTM: Pre-commit installation step is correct.The pre-commit installation step is straightforward and correct. However, for better reproducibility, consider pinning the pre-commit version or using a requirements file.
You could update the step as follows:
- name: Install pre-commit - run: pip install pre-commit + run: pip install pre-commit==3.3.3 # Replace with the desired versionAlternatively, you could create a
requirements.txtfile with the pinned version and install from it.packages/core/installer/images/cozystack.json (2)
18-36: LGTM! Consider multi-arch build support.The invocation section is well-defined:
- It correctly specifies the Dockerfile as the configuration source.
- The frontend and local parameters are appropriately set.
- The target platform is explicitly defined as "linux/amd64".
While the current configuration is correct, consider supporting multi-architecture builds in the future. This could be achieved by specifying multiple platforms in the environment section or by using Docker's buildx feature for multi-arch builds.
40-41: LGTM! Consider using specific version tags.The image digest and name are correctly specified:
- The container image digest is provided, which is crucial for image verification.
- The image name follows the standard Docker naming convention.
While using the "latest" tag is common, it's generally recommended to use specific version tags (e.g., semantic versioning) for production images. This practice ensures better traceability and prevents unintended updates. Consider implementing a versioning strategy for your images.
packages/core/installer/Makefile (1)
25-37: LGTM: Robust version checking foryq.The new
check-yq-versiontarget is a well-implemented safeguard to ensure the correct version ofyqis installed. It properly handles cases whereyqis not installed and when the version is less than required.Consider clarifying the behavior for versions greater than the required version. If newer versions are acceptable, you might want to adjust the comparison logic to allow them:
- if [ "$$(printf '%s\n' "$(YQ_VERSION)" "$$current_version" | sort -V | head -n1)" = "$(YQ_VERSION)" ]; then \ + if [ "$$(printf '%s\n' "$(YQ_VERSION)" "$$current_version" | sort -V | head -n1)" = "$(YQ_VERSION)" ]; then \ echo "Greater than or equal to $(YQ_VERSION)" ; \ else \ - echo "$(RED)ERROR: yq version less than $(YQ_VERSION)$(RESET)" ; \ + echo "$(RED)ERROR: yq version $(YQ_VERSION) is required, but $$current_version is installed$(RESET)" ; \ exit 1 ; \ fiThis change would make it clear that the exact version or newer is acceptable, which is typically the case for tool dependencies.
packages/apps/postgres/templates/init-script.yaml (4)
Line range hint
66-98: Robust implementation for database and role managementThe changes in this segment significantly improve the database and role creation process:
- Checks for existing databases and roles prevent duplication.
- Comments on databases and roles aid in management.
- Comprehensive privilege granting covers all necessary database objects.
These enhancements contribute to a more maintainable and secure database setup.
Consider adding error handling for the
DOblock to catch and log any unexpected issues during schema ownership changes and privilege assignments.
Line range hint
107-125: Well-implemented read-only privilegesThis segment effectively sets up read-only privileges:
- Covers all relevant object types: tables, sequences, and functions.
- Correctly restricts privileges to read-only operations.
- Sets both current and default privileges, maintaining consistency for future objects.
The implementation is robust and follows best practices for role-based access control.
Consider extracting the privilege-granting logic for both admin and readonly roles into a separate function to reduce code duplication. This would make the script more maintainable and less prone to inconsistencies if privileges need to be updated in the future.
Line range hint
125-152: Excellent implementation of automatic schema privilege managementThis event trigger function is a proactive approach to maintaining consistent privileges:
- Automatically handles newly created schemas.
- Sets appropriate ownership and privileges for both admin and readonly roles.
- Ensures that all future schemas will have the correct security settings.
This implementation significantly enhances the overall security and consistency of the database.
Consider adding a
RAISE NOTICEstatement at the beginning of the function to log when it's triggered. This could aid in debugging and auditing schema creations.RAISE NOTICE 'Auto-granting privileges for new schema: %', obj.object_identity;
Line range hint
161-190: Flexible and comprehensive role assignment and extension managementThis segment effectively handles role assignments and extension creation:
- Dynamically assigns or revokes roles based on Helm values.
- Ensures that role assignments are always up-to-date.
- Conditionally creates extensions, allowing for easy customization.
The implementation provides great flexibility for database setup through Helm values.
Consider adding a comment or log statement before the extension creation loop to indicate which database is being processed. This could be helpful for debugging in multi-database setups:
RAISE NOTICE 'Creating extensions for database: %', '{{ $database }}';
🛑 Comments failed to post (3)
.github/workflows/pre-commit.yml (2)
20-21: 🛠️ Refactor suggestion
Improve pre-commit hook execution for better coverage.
The current approach efficiently targets only changed YAML and Markdown files. However, it might miss some checks if files are renamed or deleted. Consider the following improvements:
- Use a more robust grep pattern.
- Include the full pre-commit run as a separate step for comprehensive checks.
Here's a suggested improvement:
- name: Run pre-commit hooks on changed files run: git diff --name-only HEAD^ | grep -E '\.(yaml|yml|md)$' | xargs -r pre-commit run --files - name: Run all pre-commit hooks run: pre-commit run --all-filesThis change will:
- Improve the grep pattern to catch both
.yamland.ymlextensions.- Use
-rwith xargs to handle cases where no files match.- Add a separate step to run all pre-commit hooks, ensuring comprehensive checks.
9-15:
⚠️ Potential issueUpdate action versions and consider using a newer Python version.
The current setup uses outdated versions of actions and an older Python version. Consider the following updates:
- Update
actions/checkoutto v3 or v4.- Update
actions/setup-pythonto v4.- Use a more recent Python version, such as 3.11 or 3.12.
Here's the suggested change:
- name: Checkout code - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: Set up Python - uses: actions/setup-python@v2 + uses: actions/setup-python@v4 with: - python-version: '3.8' + python-version: '3.11'These updates will ensure you're using the latest features and security improvements in both the actions and Python itself.
📝 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.- name: Checkout code uses: actions/checkout@v3 - name: Set up Python uses: actions/setup-python@v4 with: python-version: '3.11'🧰 Tools
🪛 actionlint
10-10: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
13-13: the runner of "actions/setup-python@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
packages/core/installer/Makefile (1)
45-45: 💡 Codebase verification
Incomplete Removal of ARM64 Support Across the Codebase
ARM64 platform support has been removed from
packages/core/installer/Makefile, but references tolinux/arm64still exist in other parts of the codebase:
packages/core/testing/Makefilepackages/apps/kubernetes/images/ubuntu-container-disk/DockerfileThis inconsistency may lead to build or runtime issues on ARM-based systems. Please ensure that ARM64 support is fully removed or consistently maintained across all relevant files.
🔗 Analysis chain
Please clarify the removal of ARM64 support.
The platform specification for the cozystack image build has been changed to only include
linux/amd64, removing support forlinux/arm64. This change might limit the usability of the image on ARM-based systems.Could you please clarify the reasoning behind this change? If ARM support is no longer needed, it would be helpful to document this decision. If there were issues with multi-architecture builds, consider if these could be addressed instead of removing ARM support entirely.
To verify the impact of this change, you can run the following command to check for any ARM-specific code or configurations in the project:
This will help identify any other places in the project that might need to be updated due to the removal of ARM64 support.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for ARM-specific code or configurations rg -i 'arm|aarch64' --type yaml --type dockerfile --type makefileLength of output: 372
Script:
#!/bin/bash # Corrected script to search for ARM-specific code or configurations rg -i 'arm|aarch64' --type yaml --type dockerfile --type makefileLength of output: 102
Script:
#!/bin/bash # Corrected script to search for ARM-specific code or configurations # Adjusted to include Dockerfile variations using -g instead of --type rg -i 'arm|aarch64' --type yaml --type makefile -g Dockerfile*Length of output: 99
Script:
#!/bin/bash # Corrected script to search for ARM-specific code or configurations rg -i 'arm|aarch64' --type yaml -g 'Dockerfile*' -g 'Makefile*'Length of output: 560
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (4)
.github/workflows/pre-commit.yml (2)
6-7: Consider specifying a specific Ubuntu version.While using
ubuntu-latestis generally good, it's recommended to specify a particular Ubuntu version (e.g.,ubuntu-22.04) to ensure consistent behavior across different runs and to avoid potential issues if the latest version changes unexpectedly.jobs: pre-commit: - runs-on: ubuntu-latest + runs-on: ubuntu-22.04
17-18: Consider using a requirements file or pinning the pre-commit version.While the current installation method works, using a requirements file or pinning the version of pre-commit can ensure consistency across different environments and prevent potential issues with future updates.
Example using a requirements file:
- name: Install pre-commit - run: pip install pre-commit + run: | + echo "pre-commit==3.3.3" > requirements.txt + pip install -r requirements.txtOr, if you prefer to keep it in one line:
- name: Install pre-commit - run: pip install pre-commit + run: pip install pre-commit==3.3.3README.md (2)
9-9: Remove unnecessary trailing spaces and approve bullet point formatting.The changes to the bullet point spacing in the Use-Cases section (lines 21, 24, 27) improve consistency and readability. However, there are unnecessary trailing spaces added at the end of lines 9 and 17. These trailing spaces don't affect the rendered output but are generally considered bad practice in Markdown.
Please remove the trailing spaces from lines 9 and 17. You can use the following diff to make these changes:
-[](https://github.com/aenix-io/cozystack) +[](https://github.com/aenix-io/cozystack) -You can use Cozystack to build your own cloud or to provide a cost-effective development environments. +You can use Cozystack to build your own cloud or to provide cost-effective development environments.Also applies to: 17-17, 21-27
21-27: Improve grammar by adding missing articles.The static analysis tool has identified some grammatical improvements that can be made to enhance the readability of the Use-Cases section. Please consider the following changes:
Apply this diff to improve the grammar:
-* [**Using Cozystack to build public cloud**](https://cozystack.io/docs/use-cases/public-cloud/) +* [**Using Cozystack to build a public cloud**](https://cozystack.io/docs/use-cases/public-cloud/) You can use Cozystack as backend for a public cloud -* [**Using Cozystack to build private cloud**](https://cozystack.io/docs/use-cases/private-cloud/) +* [**Using Cozystack to build a private cloud**](https://cozystack.io/docs/use-cases/private-cloud/) -You can use Cozystack as platform to build a private cloud powered by Infrastructure-as-Code approach +You can use Cozystack as a platform to build a private cloud powered by an Infrastructure-as-Code approach -* [**Using Cozystack as Kubernetes distribution**](https://cozystack.io/docs/use-cases/kubernetes-distribution/) +* [**Using Cozystack as a Kubernetes distribution**](https://cozystack.io/docs/use-cases/kubernetes-distribution/)These changes will improve the grammatical correctness of the sentences without altering their meaning.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~21-~21: You might be missing the article “a” here.
Context: ...se-Cases * [Using Cozystack to build public cloud](https://cozystack.io/docs/use-...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~22-~22: You might be missing the article “a” here.
Context: ...public-cloud/) You can use Cozystack as backend for a public cloud * [**Using Cozystac...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~24-~24: You might be missing the article “a” here.
Context: ...ic cloud * [Using Cozystack to build private cloud](https://cozystack.io/docs/use-...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~25-~25: You might be missing the article “a” here.
Context: ...rivate-cloud/) You can use Cozystack as platform to build a private cloud powered by Inf...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~25-~25: You might be missing the article “an” here.
Context: ...orm to build a private cloud powered by Infrastructure-as-Code approach * [**Using Cozystack ...(AI_EN_LECTOR_MISSING_DETERMINER_AN)
[uncategorized] ~27-~27: You might be missing the article “a” here.
Context: ...-Code approach * [Using Cozystack as Kubernetes distribution](https://cozystack.io/do...(AI_EN_LECTOR_MISSING_DETERMINER_A)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
- .github/workflows/pre-commit.yml (1 hunks)
- .pre-commit-config.yaml (1 hunks)
- README.md (3 hunks)
- packages/apps/README.md (1 hunks)
- packages/apps/postgres/Chart.yaml (1 hunks)
- packages/apps/postgres/README.md (1 hunks)
- packages/apps/postgres/templates/init-script.yaml (6 hunks)
- packages/apps/versions_map (1 hunks)
- packages/core/installer/Makefile (2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/apps/postgres/Chart.yaml
🚧 Files skipped from review as they are similar to previous changes (5)
- .pre-commit-config.yaml
- packages/apps/README.md
- packages/apps/postgres/README.md
- packages/apps/postgres/templates/init-script.yaml
- packages/core/installer/Makefile
🧰 Additional context used
🪛 actionlint
.github/workflows/pre-commit.yml
10-10: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
13-13: the runner of "actions/setup-python@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 LanguageTool
README.md
[uncategorized] ~21-~21: You might be missing the article “a” here.
Context: ...se-Cases * [Using Cozystack to build public cloud](https://cozystack.io/docs/use-...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~22-~22: You might be missing the article “a” here.
Context: ...public-cloud/) You can use Cozystack as backend for a public cloud * [**Using Cozystac...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~24-~24: You might be missing the article “a” here.
Context: ...ic cloud * [Using Cozystack to build private cloud](https://cozystack.io/docs/use-...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~25-~25: You might be missing the article “a” here.
Context: ...rivate-cloud/) You can use Cozystack as platform to build a private cloud powered by Inf...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~25-~25: You might be missing the article “an” here.
Context: ...orm to build a private cloud powered by Infrastructure-as-Code approach * [**Using Cozystack ...(AI_EN_LECTOR_MISSING_DETERMINER_AN)
[uncategorized] ~27-~27: You might be missing the article “a” here.
Context: ...-Code approach * [Using Cozystack as Kubernetes distribution](https://cozystack.io/do...(AI_EN_LECTOR_MISSING_DETERMINER_A)
🔇 Additional comments (3)
.github/workflows/pre-commit.yml (1)
1-3: LGTM: Workflow name and trigger are well-defined.The workflow name "Pre-Commit Checks" is clear and descriptive. Triggering on both push and pull request events ensures that the checks are run at appropriate times during the development process.
packages/apps/versions_map (1)
55-56: LGTM: Postgres versions updated correctlyThe changes to the postgres versions look good:
- Version 0.7.0 is now associated with a specific commit (dc9d8bb).
- A new version 0.7.1 has been added and marked as HEAD.
These updates follow the established versioning pattern in the file and maintain consistency with other package versions.
README.md (1)
Line range hint
1-67: Overall approval with minor suggestions for improvement.The changes to the README.md file primarily consist of formatting adjustments that improve consistency, particularly in the bullet point spacing. These changes enhance the overall readability of the document without altering its content or structure.
I've suggested a few minor improvements:
- Removing unnecessary trailing spaces.
- Adding missing articles to improve grammar in the Use-Cases section.
Once these small adjustments are made, the README will be in excellent shape. Great job on maintaining and improving the project documentation!
🧰 Tools
🪛 LanguageTool
[uncategorized] ~21-~21: You might be missing the article “a” here.
Context: ...se-Cases * [Using Cozystack to build public cloud](https://cozystack.io/docs/use-...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~22-~22: You might be missing the article “a” here.
Context: ...public-cloud/) You can use Cozystack as backend for a public cloud * [**Using Cozystac...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~24-~24: You might be missing the article “a” here.
Context: ...ic cloud * [Using Cozystack to build private cloud](https://cozystack.io/docs/use-...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~25-~25: You might be missing the article “a” here.
Context: ...rivate-cloud/) You can use Cozystack as platform to build a private cloud powered by Inf...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~25-~25: You might be missing the article “an” here.
Context: ...orm to build a private cloud powered by Infrastructure-as-Code approach * [**Using Cozystack ...(AI_EN_LECTOR_MISSING_DETERMINER_AN)
[uncategorized] ~27-~27: You might be missing the article “a” here.
Context: ...-Code approach * [Using Cozystack as Kubernetes distribution](https://cozystack.io/do...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~28-~28: You might be missing the article “a” here.
Context: ...distribution/) You can use Cozystack as Kubernetes distribution for Bare Metal ## Screens...(AI_EN_LECTOR_MISSING_DETERMINER_A)
.github/workflows/pre-commit.yml
Outdated
| - name: Checkout code | ||
| uses: actions/checkout@v2 | ||
|
|
||
| - name: Set up Python | ||
| uses: actions/setup-python@v2 | ||
| with: | ||
| python-version: '3.8' |
There was a problem hiding this comment.
Update action versions and Python version.
The current versions of the actions and Python are outdated. Consider updating them to the latest stable versions for improved features, performance, and security.
Apply these changes:
- name: Checkout code
- uses: actions/checkout@v2
+ uses: actions/checkout@v3
- name: Set up Python
- uses: actions/setup-python@v2
+ uses: actions/setup-python@v4
with:
- python-version: '3.8'
+ python-version: '3.11'📝 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.
| - name: Checkout code | |
| uses: actions/checkout@v2 | |
| - name: Set up Python | |
| uses: actions/setup-python@v2 | |
| with: | |
| python-version: '3.8' | |
| - name: Checkout code | |
| uses: actions/checkout@v3 | |
| - name: Set up Python | |
| uses: actions/setup-python@v4 | |
| with: | |
| python-version: '3.11' |
🧰 Tools
🪛 actionlint
10-10: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
13-13: the runner of "actions/setup-python@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/pre-commit.yml
Outdated
| - name: Run pre-commit hooks | ||
| run: git diff --name-only HEAD^ | grep '\.yaml\|\.md' | xargs pre-commit run --files |
There was a problem hiding this comment.
Improve robustness of pre-commit hook execution.
The current implementation might miss some files if there are multiple commits in the PR, and it could fail if there are no matching files. Consider these improvements:
- Use
git diffagainst the base branch of the PR to catch all changes. - Use
grep -Efor extended regex and make it optional with|| true. - Add quotes around
${{ github.event.pull_request.base.sha }}to handle cases where it might be empty.
Apply this change:
- name: Run pre-commit hooks
- run: git diff --name-only HEAD^ | grep '\.yaml\|\.md' | xargs pre-commit run --files
+ run: |
+ git diff --name-only ${{ github.event.pull_request.base.sha || 'HEAD^' }} |
+ grep -E '\.ya?ml$|\.md$' || true |
+ xargs -r pre-commit run --filesThis change will:
- Compare against the PR base or the previous commit if not in a PR context.
- Use extended regex to match YAML (both .yml and .yaml) and Markdown files.
- Use
xargs -rto avoid running pre-commit with no arguments if no files match.
📝 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.
| - name: Run pre-commit hooks | |
| run: git diff --name-only HEAD^ | grep '\.yaml\|\.md' | xargs pre-commit run --files | |
| - name: Run pre-commit hooks | |
| run: | | |
| git diff --name-only ${{ github.event.pull_request.base.sha || 'HEAD^' }} | | |
| grep -E '\.ya?ml$|\.md$' || true | | |
| xargs -r pre-commit run --files |
klinch0
force-pushed
the
bugfix/fix_postgres_job
branch
from
0dc00b2 to
3e56af7
Compare
README.md
Outdated
| * [**Using Cozystack to build public cloud**](https://cozystack.io/docs/use-cases/public-cloud/) | ||
| * [**Using Cozystack to build public cloud**](https://cozystack.io/docs/use-cases/public-cloud/) | ||
| You can use Cozystack as backend for a public cloud | ||
|
|
||
| * [**Using Cozystack to build private cloud**](https://cozystack.io/docs/use-cases/private-cloud/) | ||
| * [**Using Cozystack to build private cloud**](https://cozystack.io/docs/use-cases/private-cloud/) | ||
| You can use Cozystack as platform to build a private cloud powered by Infrastructure-as-Code approach | ||
|
|
||
| * [**Using Cozystack as Kubernetes distribution**](https://cozystack.io/docs/use-cases/kubernetes-distribution/) | ||
| * [**Using Cozystack as Kubernetes distribution**](https://cozystack.io/docs/use-cases/kubernetes-distribution/) | ||
| You can use Cozystack as Kubernetes distribution for Bare Metal |
There was a problem hiding this comment.
Markdown syntax is very tricky, these two examples are rendered differently. Those are there for reason
-
with
-
without
README.md
Outdated
| Versioning adheres to the [Semantic Versioning](http://semver.org/) principles. | ||
| Versioning adheres to the [Semantic Versioning](http://semver.org/) principles. | ||
| A full list of the available releases is available in the GitHub repository's [Release](https://github.com/aenix-io/cozystack/releases) section. |
README.md
Outdated
| Cozystack is licensed under Apache 2.0. | ||
| Cozystack is licensed under Apache 2.0. |
packages/core/installer/Makefile
Outdated
| check-yq-version: | ||
| @current_version=$$(yq -V | grep -oP 'v[0-9]+\.[0-9]+\.[0-9]+') ; \ | ||
| if [ -z "$$current_version" ]; then \ | ||
| echo "yq is not installed or version cannot be determined." ; \ | ||
| exit 1 ; \ | ||
| fi ; \ | ||
| echo "Current yq version: $$current_version" ; \ | ||
| if [ "$$(printf '%s\n' "$(YQ_VERSION)" "$$current_version" | sort -V | head -n1)" = "$(YQ_VERSION)" ]; then \ | ||
| echo "Greater than or equal to $(YQ_VERSION)" ; \ | ||
| else \ | ||
| echo "$(RED)ERROR: yq version less than $(YQ_VERSION)$(RESET)" ; \ | ||
| exit 1 ; \ | ||
| fi |
There was a problem hiding this comment.
(nit) It might be be better to place it into common hack/pre-checks.sh and invoke it everytime before the building any image.
klinch0
force-pushed
the
bugfix/fix_postgres_job
branch
from
3e56af7 to
8aa3f8c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
hack/pre-checks.sh (2)
1-5: Add a brief description and usage instructions.The script setup looks good, with appropriate shebang and constant definitions. However, it would be beneficial to add a brief description of the script's purpose and any usage instructions at the beginning of the file. This will help other developers understand the script's functionality and how to use it.
Consider adding something like this at the beginning of the file:
#!/bin/bash # Description: This script checks the installed version of yq against a required version. # Usage: ./pre-checks.sh YQ_VERSION="v4.35.1" RED='\033[31m' RESET='\033[0m'
7-21: Function looks good with minor suggestions for improvement.The
check-yq-version()function is well-implemented with clear logic and error handling. Here are a few suggestions for improvement:
- The error message for when
yqis not installed could be more specific.- Consider adding a check for the
yqcommand's existence before trying to get its version.- The success message could be more explicit about the check passing.
Here's a suggested improvement:
check-yq-version() { + if ! command -v yq &> /dev/null; then + echo -e "${RED}ERROR: yq is not installed. Please install yq version $YQ_VERSION or later.${RESET}" + exit 1 + fi + current_version=$(yq -V | grep -oP 'v[0-9]+\.[0-9]+\.[0-9]+') if [ -z "$current_version" ]; then - echo "yq is not installed or version cannot be determined." + echo -e "${RED}ERROR: yq version cannot be determined.${RESET}" exit 1 fi echo "Current yq version: $current_version" if [ "$(printf '%s\n' "$YQ_VERSION" "$current_version" | sort -V | head -n1)" = "$YQ_VERSION" ]; then - echo "Greater than or equal to $YQ_VERSION" + echo "yq version check passed. Current version ($current_version) is greater than or equal to required version ($YQ_VERSION)." else echo -e "${RED}ERROR: yq version less than $YQ_VERSION${RESET}" exit 1 fi }.github/workflows/pre-commit.yml (1)
20-35: Good improvements, with a minor suggestion for file matching.The pre-commit hook execution step has been significantly improved:
- It correctly handles different base branch names (main or master).
- It properly manages cases where there might be no matching files.
- The implementation is more robust than the previous version.
These improvements address the concerns raised in the past review comment.
However, there's a minor suggestion for enhancement:
Consider updating the file matching to include
.ymlfiles as well:- files=$(git ls-files '*.yaml' '*.md') + files=$(git ls-files '*.yaml' '*.yml' '*.md')and
- files=$(git diff --name-only "$base_commit" -- '*.yaml' '*.md') + files=$(git diff --name-only "$base_commit" -- '*.yaml' '*.yml' '*.md')This change ensures that all YAML files (both .yaml and .yml extensions) are included in the pre-commit checks.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
- .github/workflows/pre-commit.yml (1 hunks)
- .pre-commit-config.yaml (1 hunks)
- hack/pre-checks.sh (1 hunks)
- packages/apps/README.md (1 hunks)
- packages/apps/postgres/Chart.yaml (1 hunks)
- packages/apps/postgres/README.md (1 hunks)
- packages/apps/postgres/templates/init-script.yaml (6 hunks)
- packages/apps/versions_map (1 hunks)
- packages/core/installer/Makefile (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- .pre-commit-config.yaml
- packages/apps/README.md
- packages/apps/postgres/Chart.yaml
- packages/apps/postgres/README.md
- packages/apps/postgres/templates/init-script.yaml
- packages/apps/versions_map
- packages/core/installer/Makefile
🧰 Additional context used
🔇 Additional comments (4)
hack/pre-checks.sh (1)
23-23: Function call looks good.The
check-yq-versionfunction is correctly called at the end of the script. This ensures that the version check is performed when the script is executed..github/workflows/pre-commit.yml (3)
1-3: LGTM: Workflow name and trigger are well-defined.The workflow name "Pre-Commit Checks" is clear and descriptive. Triggering on both push and pull request events ensures that the checks are run at appropriate times during the development process.
5-7: LGTM: Job setup is appropriate.The job name "pre-commit" is concise and descriptive. Using a specific Ubuntu version (22.04) ensures consistency and reproducibility of the environment across runs.
9-18: LGTM: Setup steps are well-defined and use up-to-date versions.The setup steps are correctly implemented:
- Using
actions/checkout@v3for code checkout.- Setting up Python 3.11 with
actions/setup-python@v4.- Installing pre-commit using pip.
These steps address the previous review comment about updating action versions and Python version, which is a good improvement.
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes - **New Features** - Introduced a pre-commit workflow to automate checks before code merges. - Added a section in the README for testing packages locally. - **Improvements** - Enhanced PostgreSQL initialization script for better user and role management. - Updated documentation for Managed PostgreSQL Service with improved formatting and additional backup parameters. - Integrated pre-commit hooks for maintaining code quality in YAML and Markdown files. - Added a new target in the installer Makefile to run pre-checks before building images. - **Bug Fixes** - Adjusted formatting in various README files to ensure consistent presentation. - **Chores** - Updated image reference to use the latest version in configuration files. - Updated versioning for various packages in the versions map. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes
Chores