Add GitHub Actions CI workflow for Django project#75
Add GitHub Actions CI workflow for Django project#75maximprysyazhnikov wants to merge 3 commits into
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
This requires a few changes before it can be approved. The most critical issue is the location of your workflow file. It's currently at src/.github/workflows/django-ci.yml, but for GitHub Actions to detect it, it must be in the root .github/workflows/ directory. Please move the file there so the workflow can run.
Additionally, you can make the workflow more efficient and clear:
- The 'Run tests' step is redundant because the 'Generate coverage report' step already runs the tests via
coverage run. You can remove the initial test step. - To keep your workflow steps focused, please remove the
--max-complexity=10flag from the 'code style' step, as you already have a separate step for the complexity check.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| - name: Run tests | ||
| working-directory: ./src | ||
| run: | | ||
| python manage.py test |
There was a problem hiding this comment.
This step to run tests is redundant. The next step, 'Generate coverage report', already runs the tests using coverage run ... manage.py test. You can remove this entire 'Run tests' step to make your workflow more efficient.
| continue-on-error: true | ||
| run: | | ||
| flake8 src --count --select=E9,F63,F7,F82 --show-source --statistics | ||
| flake8 src --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics |
There was a problem hiding this comment.
This flake8 command includes a complexity check (--max-complexity=10). According to the requirements, there should be a separate step for complexity checking, which you have implemented correctly. To keep the steps focused on their specific tasks, you should remove the complexity check from this 'code style' step.
- Move workflow file to root .github/workflows directory - Remove duplicate workflow from src/.github - Remove redundant 'Run tests' step (tests already run in coverage step) - Remove complexity check from code style step (separate step exists)
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Excellent progress on the workflow! You've successfully moved the file to the correct .github/workflows/ directory, and the logic inside is spot-on.
However, a couple of cleanup items are needed before this can be approved.
- The old, duplicate workflow file still exists at
src/.github/workflows/django-ci.yml. Please remove this file to keep the repository clean. - The
.ideadirectory, which contains IDE-specific settings, has been committed. This should be removed, and.idea/should be added to your root.gitignorefile to prevent it from being committed in the future.
Once you've addressed these repository cleanup tasks, your work will be complete!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| @@ -0,0 +1,72 @@ | |||
| name: Django CI | |||
There was a problem hiding this comment.
This workflow file is in the src/.github/workflows/ directory. For GitHub Actions to detect and run it, it must be located in the root .github/workflows/ directory of your repository. Please ensure the workflow file is only present at the correct path.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving this step to help you move forward, but there are a couple of important cleanup tasks left to complete. Great job implementing the main workflow file and adding .idea/ to your .gitignore! The core logic is correct.
However, two issues from the previous review remain:
- The duplicate workflow file at
src/.github/workflows/django-ci.ymlstill needs to be deleted. - Although
.idea/is in your.gitignore, Git is still tracking it. To fix this, rungit rm -r --cached .ideain your terminal and commit the change to untrack the directory.
Once you've taken care of those two items, your repository will be perfectly clean. Keep up the great work!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| name: Django CI | ||
|
|
||
| on: | ||
| push: | ||
| branches: | ||
| - main | ||
| - develop | ||
| pull_request: | ||
| branches: | ||
| - main | ||
|
|
||
| run-name: "CI run by ${{ github.actor }} for commit ${{ github.sha }}" | ||
|
|
||
| jobs: | ||
| python-ci: | ||
| runs-on: ubuntu-latest | ||
|
|
||
| env: | ||
| PYTHON_VERSION: '3.9' | ||
|
|
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Set up Python ${{ env.PYTHON_VERSION }} | ||
| uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: ${{ env.PYTHON_VERSION }} | ||
|
|
||
| - name: Install dependencies | ||
| run: | | ||
| python -m pip install --upgrade pip | ||
| if [ -f requirements.txt ]; then pip install -r requirements.txt; fi | ||
| if [ -f src/requirements.txt ]; then pip install -r src/requirements.txt; fi | ||
| pip install coverage flake8 | ||
|
|
||
| - name: Run database migrations | ||
| working-directory: ./src | ||
| run: | | ||
| python manage.py migrate | ||
|
|
||
| - name: Generate coverage report | ||
| working-directory: ./src | ||
| run: | | ||
| coverage run --source='.' manage.py test | ||
| coverage xml | ||
|
|
||
| - name: Display coverage report | ||
| working-directory: ./src | ||
| run: | | ||
| coverage report | ||
|
|
||
| - name: Check code style with flake8 | ||
| continue-on-error: true | ||
| run: | | ||
| flake8 src --count --select=E9,F63,F7,F82 --show-source --statistics | ||
| flake8 src --count --exit-zero --max-line-length=127 --statistics | ||
|
|
||
| - name: Check code complexity with flake8 | ||
| continue-on-error: true | ||
| run: | | ||
| flake8 src --count --exit-zero --max-complexity=10 --statistics | ||
|
|
||
| - name: Upload Python code as artifact | ||
| uses: actions/upload-artifact@v3 | ||
| with: | ||
| name: python-code-${{ github.sha }} | ||
| path: | | ||
| src/**/*.py | ||
| src/requirements.txt | ||
| src/manage.py | ||
| retention-days: 30 No newline at end of file |
There was a problem hiding this comment.
This file is a duplicate of the workflow located in the root .github/workflows/ directory. As per the previous review, this file should be deleted to avoid confusion and keep the repository clean. GitHub Actions will only recognize and run workflows from the root .github directory.
2 participants
No description provided.