Update storing-artifacts.md #97
Conversation
There was a problem hiding this comment.
LGTM! And as you said so yourself @ncsokas , we might have to look at the other exercises to align with the best practice structure.
But lets wait with the merge until the rest of the exercises is updated 😄
There was a problem hiding this comment.
While I think this PR is good improvement and should be kept for now, it seems like something that should have been a basic premise for the whole exercise. I think the exercise should be re-written so that it revolves around extracting the unit-test step into its own job. The linting may be added as a bonus exercise if necessary, but seems like it should be it's own, unrelated exercise.
If we keep the linting, we should consider configuring it to run only the necessary linters instead of simply disabling errors. It can be configured with VALIDATE_[LANGUAGE] variables.
|
|
||
| ## Extra Exercise: Reorganizing Your Workflow | ||
|
|
||
| However, while the current setup is a great start for understanding artifacts, but we can make it more efficient and align it with best practices. |
There was a problem hiding this comment.
| However, while the current setup is a great start for understanding artifacts, but we can make it more efficient and align it with best practices. | |
| While the current workflow is a great start for understanding artifacts, we can make it more efficient and align it with best practices. |
|
|
||
| Now, create a completely new job for testing. This job will run after the `Build` job is finished and will test the artifact that was created. | ||
|
|
||
| Add a new job named `Test`. |
There was a problem hiding this comment.
| Add a new job named `Test`. | |
| Add a new job named `Test` that will be executed inside a `gradle:6-jdk11` `container` which in turn `runs-on` an `ubuntu-latest` agent |
| with: | ||
| name: code | ||
| path: . | ||
| include-hidden-files: true |
There was a problem hiding this comment.
include-hidden-files is only valid for the upload action
| with: | |
| name: code | |
| path: . | |
| include-hidden-files: true | |
| with: | |
| name: code | |
| path: . |
|
|
||
| - name: Run unit tests | ||
| run: ci/unit-test-app.sh |
There was a problem hiding this comment.
File permissions are not preserved when uploaded. The script will have to be made executable in order to run. This needs to be reflected in the exercise steps as well.
| - name: Run unit tests | |
| run: ci/unit-test-app.sh | |
| - name: Ensure unit test script is executable | |
| run: chmod +x ci/unit-test-app.sh | |
| - name: Run unit tests | |
| run: ci/unit-test-app.sh |
There was a problem hiding this comment.
The docker-images exercise makes use of the code artifact. If the extra assignment is completed in storing-artifacts, the code artifact is no longer available, and the exercise will fail. Rewriting the `docker-images´ to take this into account should be part of this PR.
| - name: Upload build artifact | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: code | ||
| path: . | ||
| include-hidden-files: true |
There was a problem hiding this comment.
The exercise step says:
Change the
Upload repostep toUpload build artifact
But we're still uploading the full code. Should this upload the build folder instead?
This pull request updates the
labs/storing-artifacts.mdfile to introduce an extra exercise to reorganize the workflow for better efficiency and alignment with best practices. The changes include breaking the workflow into dedicated jobs (build,test, andlint), optimizing artifact usage, and improving parallelism.Enhancements to the tutorial:
build,test, andlint. This includes separating responsibilities and improving efficiency.Workflow improvements:
Buildjob to upload only the necessary build artifact instead of the entire source code.Testjob that runs after theBuildjob, downloads the build artifact, and executes unit tests against it.Lintingjob to run in parallel by removing the dependency on theBuildjob and replacing the artifact download step with a repository checkout step.