Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughRemoved Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub as GitHub Event
participant PublishWF as Publish Workflow
participant DetectEnvs as Detect Environments Job
participant PublishImage as Publish-image Job
participant ReusableDeploy as Reusable deploy.yml
GitHub->>PublishWF: trigger (push to main or workflow_dispatch)
PublishWF->>DetectEnvs: run environment detection
DetectEnvs-->>PublishImage: return environments (matrix)
PublishImage->>ReusableDeploy: call reusable workflow per environment
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Coverage Report
File CoverageNo changed files found. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
.github/workflows/test.yml (1)
19-21: Trailing whitespace – tiny YAML-lint failureThere are stray spaces after
branches-ignore:. They do no harm but failyamllint’s default rules.- push: - branches-ignore: [main] - + push: + branches-ignore: [main].github/workflows/publish.yml (3)
2-2: Run-name field has double space after colonYAML-lint warns: “too many spaces after colon”. Trim the surplus to keep formatting tidy.
-run-name: Publish - ${{ github.head_ref || github.ref_name }} to ${{ inputs.environment || 'All' }} by @${{ github.actor }} triggered via ${{ github.event_name }} +run-name: Publish - ${{ github.head_ref || github.ref_name }} to ${{ inputs.environment || 'All' }} by @${{ github.actor }} triggered via ${{ github.event_name }}
40-44: No newline at end of fileA missing final newline is flagged by
yamllintand can cause noisy diffs later.- secrets: inherit + secrets: inherit +
14-18: Potential empty input when triggered bypushWhen this workflow runs on
push,inputs.environmentis undefined, so the calledtest.ymlreceives an empty string.
Given the earlier fix (input notrequired), this is now legal and will default todevelopment, but the explicit empty string is confusing. Prefer passing the resolved default:- environment: '${{ inputs.environment }}' + environment: '${{ inputs.environment || \'development\' }}'Purely cosmetic, but makes intent explicit.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/featureDeploy.yml(0 hunks).github/workflows/publish.yml(2 hunks).github/workflows/security-scan.yml(1 hunks).github/workflows/test.yml(1 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/featureDeploy.yml
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: cpcundill
PR: digital-land/submit#760
File: .github/workflows/security-scan.yml:1-11
Timestamp: 2025-01-06T14:18:24.064Z
Learning: In .github/workflows/security-scan.yml, the Docker repository secret is named DEPLOY_DOCKER_REPOSITORY, and should be used as follows:
DOCKER_REPO: ${{ secrets.DEPLOY_DOCKER_REPOSITORY }}/submit:latest
.github/workflows/security-scan.yml (1)
Learnt from: cpcundill
PR: #760
File: .github/workflows/security-scan.yml:1-11
Timestamp: 2025-01-06T14:18:24.064Z
Learning: In .github/workflows/security-scan.yml, the Docker repository secret is named DEPLOY_DOCKER_REPOSITORY, and should be used as follows:
DOCKER_REPO: ${{ secrets.DEPLOY_DOCKER_REPOSITORY }}/submit:latest
.github/workflows/test.yml (1)
Learnt from: cpcundill
PR: #760
File: .github/workflows/security-scan.yml:1-11
Timestamp: 2025-01-06T14:18:24.064Z
Learning: In .github/workflows/security-scan.yml, the Docker repository secret is named DEPLOY_DOCKER_REPOSITORY, and should be used as follows:
DOCKER_REPO: ${{ secrets.DEPLOY_DOCKER_REPOSITORY }}/submit:latest
.github/workflows/publish.yml (1)
Learnt from: cpcundill
PR: #760
File: .github/workflows/security-scan.yml:1-11
Timestamp: 2025-01-06T14:18:24.064Z
Learning: In .github/workflows/security-scan.yml, the Docker repository secret is named DEPLOY_DOCKER_REPOSITORY, and should be used as follows:
DOCKER_REPO: ${{ secrets.DEPLOY_DOCKER_REPOSITORY }}/submit:latest
🪛 actionlint (1.7.7)
.github/workflows/test.yml
10-10: input "environment" of workflow_call event has the default value "development", but it is also required. if an input is marked as required, its default value will never be used
(events)
11-11: invalid value "environment" for input type of workflow_call event. it must be one of "boolean", "number", or "string"
(syntax-check)
🪛 YAMLlint (1.37.1)
.github/workflows/test.yml
[error] 21-21: trailing spaces
(trailing-spaces)
.github/workflows/publish.yml
[warning] 2-2: too many spaces after colon
(colons)
[error] 44-44: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (1)
.github/workflows/security-scan.yml (1)
9-12: Confirm secret value – risk of duplicate or missing image tag/path
DOCKER_REPOcurrently resolves to the raw value ofsecrets.DEPLOY_DOCKER_REPOSITORY, while the laterdocker pullappends:${DOCKER_APPLICATION_TAG}.
Past repos store the secret as<account>.dkr.ecr.<region>.amazonaws.com/submit, i.e. without the tag.
If instead the secret already contains/submit:latest, the pull command will try to fetch…/submit:latest:latestand fail. Likewise, if the secret omits/submitthe pull will fetch the repository root (invalid).Please double-check the exact secret contents and, if necessary, standardise with either of the two common patterns:
- DOCKER_REPO: ${{ secrets.DEPLOY_DOCKER_REPOSITORY }} + # Option A – secret contains just the registry hostname + DOCKER_REPO: ${{ secrets.DEPLOY_DOCKER_REPOSITORY }}/submit + + # Option B – secret already contains the repository path **and** tag + # DOCKER_REPO: ${{ secrets.DEPLOY_DOCKER_REPOSITORY }}Consistency with other repos avoids surprises when rotating credentials.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/test.yml (1)
8-11:required: truemakes thedefaultunusable – remove one of them
actionlintstill complains: when an input is marked asrequired: true, itsdefaultis ignored. Droprequired(or thedefault) so callers can omit the input and fall back to “development”.- description: Which environment's containers should be used? - required: true - default: 'development' - type: string + description: Which environment's containers should be used? (e.g. development, staging, production) + default: 'development' # keep default + type: string # remove `required`
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
19-21: Trailing whitespace violates YAML-lintLine 21 ends with a stray space. It’s harmless at runtime but fails linting and clutters diffs—please trim it.
- branches-ignore: [main]␠ + branches-ignore: [main]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/test.yml(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: cpcundill
PR: digital-land/submit#760
File: .github/workflows/security-scan.yml:1-11
Timestamp: 2025-01-06T14:18:24.064Z
Learning: In .github/workflows/security-scan.yml, the Docker repository secret is named DEPLOY_DOCKER_REPOSITORY, and should be used as follows:
DOCKER_REPO: ${{ secrets.DEPLOY_DOCKER_REPOSITORY }}/submit:latest
📚 Learning: in .github/workflows/security-scan.yml, the docker repository secret is named deploy_docker_reposito...
Learnt from: cpcundill
PR: digital-land/submit#760
File: .github/workflows/security-scan.yml:1-11
Timestamp: 2025-01-06T14:18:24.064Z
Learning: In .github/workflows/security-scan.yml, the Docker repository secret is named DEPLOY_DOCKER_REPOSITORY, and should be used as follows:
DOCKER_REPO: ${{ secrets.DEPLOY_DOCKER_REPOSITORY }}/submit:latest
Applied to files:
.github/workflows/test.yml
🪛 YAMLlint (1.37.1)
.github/workflows/test.yml
[error] 21-21: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/test.yml (1)
8-10: Correctedenvironmentinput – previous actionlint error resolvedChanging the type to
stringand droppingrequired: truefixes the validation problem flagged earlier.
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
18-20: Remove trailing spaces afterbranches-ignoreYAMLlint flags trailing whitespace on Line 20. Cleaning it up avoids noisy CI warnings.
- branches-ignore: [main]␠ + branches-ignore: [main]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/test.yml(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: cpcundill
PR: digital-land/submit#760
File: .github/workflows/security-scan.yml:1-11
Timestamp: 2025-01-06T14:18:24.064Z
Learning: In .github/workflows/security-scan.yml, the Docker repository secret is named DEPLOY_DOCKER_REPOSITORY, and should be used as follows:
DOCKER_REPO: ${{ secrets.DEPLOY_DOCKER_REPOSITORY }}/submit:latest
📚 Learning: in .github/workflows/security-scan.yml, the docker repository secret is named deploy_docker_reposito...
Learnt from: cpcundill
PR: digital-land/submit#760
File: .github/workflows/security-scan.yml:1-11
Timestamp: 2025-01-06T14:18:24.064Z
Learning: In .github/workflows/security-scan.yml, the Docker repository secret is named DEPLOY_DOCKER_REPOSITORY, and should be used as follows:
DOCKER_REPO: ${{ secrets.DEPLOY_DOCKER_REPOSITORY }}/submit:latest
Applied to files:
.github/workflows/test.yml
🪛 YAMLlint (1.37.1)
.github/workflows/test.yml
[error] 20-20: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (1)
.github/workflows/test.yml (1)
2-2: Run-name enhancement looks goodIncluding branch/ref, actor and event makes runs easier to identify.
No further action required.
eveleighoj
force-pushed
the
feat/gh-workflows
branch
from
0749615 to
3c0eb39
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
.github/workflows/publish.yml (1)
15-18: Passing an empty string overrides the default in the reusable workflow
run-testsforwardswith: environment: '${{ inputs.environment }}'For a push on
main,inputs.environmentis undefined, which resolves to an empty string ('').
GitHub treats this as “value supplied”, so the default'development'intest.ymlis bypassed and the called workflow receives an empty string.If the intention is to get the default when no input is provided, inject the fallback explicitly:
- environment: '${{ inputs.environment }}' + environment: '${{ inputs.environment || '' }}'or rename the input (
deploy_envetc.) and omit it entirely when unset.
🧹 Nitpick comments (5)
.github/workflows/publish.yml (2)
2-2: Remove the double-space after the colon inrun-nameYAMLLint flags Line 2 (
too many spaces after colon).
While harmless at runtime, it trips static-analysis and review tooling.-run-name: Publish - ${{ github.head_ref || github.ref_name }} to ${{ inputs.environment || 'All' }} by @${{ github.actor }} triggered via ${{ github.event_name }} +run-name: Publish - ${{ github.head_ref || github.ref_name }} to ${{ inputs.environment || 'All' }} by @${{ github.actor }} triggered via ${{ github.event_name }}
36-44: No newline at end of fileYAMLLint reports the missing terminating newline (Line 44).
Add a blank line at the end to silence linters and keep POSIX-friendly..github/workflows/test.yml (3)
8-10: Minor wording tweakA question mark in the description makes the sentence look like a prompt rather than documentation.
- description: Which environment's containers should be used? + description: Which environment's containers should be used
18-20: Strip trailing whitespaceLine 20 has trailing spaces which fail stricter linters.
- branches-ignore: [main]␠␠ + branches-ignore: [main]
24-27: Hard-coding the job-levelenvironmentties tests to one env onlyThe job is forced to
environment: development, independent of theenvironmentinput that the caller passes. If you intend the tests to run in the requested environment, wire it through:- environment: development + environment: ${{ inputs.environment || 'development' }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/featureDeploy.yml(0 hunks).github/workflows/publish.yml(2 hunks).github/workflows/security-scan.yml(1 hunks).github/workflows/test.yml(2 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/featureDeploy.yml
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/security-scan.yml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: cpcundill
PR: digital-land/submit#760
File: .github/workflows/security-scan.yml:1-11
Timestamp: 2025-01-06T14:18:24.064Z
Learning: In .github/workflows/security-scan.yml, the Docker repository secret is named DEPLOY_DOCKER_REPOSITORY, and should be used as follows:
DOCKER_REPO: ${{ secrets.DEPLOY_DOCKER_REPOSITORY }}/submit:latest
📚 Learning: in .github/workflows/security-scan.yml, the docker repository secret is named deploy_docker_reposito...
Learnt from: cpcundill
PR: digital-land/submit#760
File: .github/workflows/security-scan.yml:1-11
Timestamp: 2025-01-06T14:18:24.064Z
Learning: In .github/workflows/security-scan.yml, the Docker repository secret is named DEPLOY_DOCKER_REPOSITORY, and should be used as follows:
DOCKER_REPO: ${{ secrets.DEPLOY_DOCKER_REPOSITORY }}/submit:latest
Applied to files:
.github/workflows/publish.yml.github/workflows/test.yml
🪛 YAMLlint (1.37.1)
.github/workflows/publish.yml
[warning] 2-2: too many spaces after colon
(colons)
[error] 44-44: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/test.yml
[error] 20-20: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (1)
.github/workflows/publish.yml (1)
20-35: Consider rate-limit & pagination when listing environments
detect-environmentsmakes a singleGET /environmentscall.
Repos with >100 environments are paginated; only the first page (100) is returned.
If that scenario is even remotely possible, add simple pagination handling to avoid silent omissions.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
.github/workflows/publish.yml (1)
9-12:type: environmentis invalid forworkflow_dispatchinputsThe same schema rule flagged earlier still applies here: only
string,numberorbooleanare allowed. Using an unsupported type breaks workflow syntax and blocks manual dispatch.- environment: - type: environment - description: The environment to deploy to. + environment: + type: string + description: The environment to deploy to (e.g. development, staging, production) + default: ''
🧹 Nitpick comments (1)
.github/workflows/publish.yml (1)
44-44: Missing newline at EOFA trailing newline keeps POSIX tools happy and silences YAML linters.
- secrets: inherit +\n
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/publish.yml(2 hunks).github/workflows/test.yml(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: cpcundill
PR: digital-land/submit#760
File: .github/workflows/security-scan.yml:1-11
Timestamp: 2025-01-06T14:18:24.064Z
Learning: In .github/workflows/security-scan.yml, the Docker repository secret is named DEPLOY_DOCKER_REPOSITORY, and should be used as follows:
DOCKER_REPO: ${{ secrets.DEPLOY_DOCKER_REPOSITORY }}/submit:latest
📚 Learning: in .github/workflows/security-scan.yml, the docker repository secret is named deploy_docker_reposito...
Learnt from: cpcundill
PR: digital-land/submit#760
File: .github/workflows/security-scan.yml:1-11
Timestamp: 2025-01-06T14:18:24.064Z
Learning: In .github/workflows/security-scan.yml, the Docker repository secret is named DEPLOY_DOCKER_REPOSITORY, and should be used as follows:
DOCKER_REPO: ${{ secrets.DEPLOY_DOCKER_REPOSITORY }}/submit:latest
Applied to files:
.github/workflows/publish.yml.github/workflows/test.yml
🪛 YAMLlint (1.37.1)
.github/workflows/publish.yml
[error] 44-44: no new line character at the end of file
(new-line-at-end-of-file)
🪛 actionlint (1.7.7)
.github/workflows/test.yml
24-24: undefined variable "development". available variables are "env", "github", "inputs", "job", "matrix", "needs", "runner", "secrets", "steps", "strategy", "vars"
(expression)
2 participants
Description
Please replace this line with a brief description of the changes made.
What type of PR is this? (check all applicable)
Related Tickets & Documents
QA Instructions, Screenshots, Recordings
Before
Before screenshot here
After
After screenshot here
Added/updated tests?
We encourage you to keep the code coverage percentage at 80% and above.
QA sign off
[optional] Are there any post-deployment tasks we need to perform?
[optional] Are there any dependencies on other PRs or Work?
Summary by CodeRabbit