[OSDEV-899] Move React application to the S3 bucket#449
Conversation
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
had a problem deploying
to
Quality Environment
GitHub Actions
Failure
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vladsha-dev
left a comment
vladsha-dev
left a comment
There was a problem hiding this comment.
- Could you please describe in more detail what was done in the PR description? We need to highlight that the FE (React) and BE (Django) have been split.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
doc/release/RELEASE-NOTES.md (1)
29-30: Enhance the release note entry for OSDEV-899.The current entry could be more detailed to help users and developers better understand this significant architectural change. Consider expanding it to:
-* [OSDEV-899](https://opensupplyhub.atlassian.net/browse/OSDEV-899) - After moving the React application to the S3 bucket, we transfer the processing of part of the traffic to the CDN, which allows us to use the computing resources of the ECS cluster more efficiently. +* [OSDEV-899](https://opensupplyhub.atlassian.net/browse/OSDEV-899) - Split the React frontend (FE) and Django backend (BE) applications by moving the React application to an S3 bucket: + - Frontend static assets are now served from S3 and distributed through CloudFront CDN + - This architectural change reduces load on the ECS cluster by offloading static content delivery + - Improved scalability and performance through CDN's global edge network + - More efficient use of ECS cluster resources by focusing on API requests only
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
doc/release/RELEASE-NOTES.md(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-flake8-linter
- GitHub Check: run-fe-code-quality
- GitHub Check: run-django-code-quality
- GitHub Check: run-dd-code-quality
- GitHub Check: run-countries-code-quality
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: get-base-branch-django-cov
- GitHub Check: get-base-branch-contricleaner-cov
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
deployment/terraform/cdn.tf (1)
116-128: CloudFront Distribution – Second Origin (S3)
This origin uses ans3_origin_configwhich is correct for S3 buckets. However, note that a custom header block is still present (lines 124-127). Traditionally, when using S3 as an origin you do not need to add a custom header for authentication. If this header is not explicitly required by your infrastructure or downstream logic, consider removing it to reduce configuration complexity.Suggested diff:
resource "aws_cloudfront_origin_access_identity" "react" { comment = local.frontend_bucket_name } resource "aws_cloudfront_distribution" "cdn" { ... origin { domain_name = aws_s3_bucket.react.bucket_regional_domain_name origin_id = "originS3" s3_origin_config { origin_access_identity = aws_cloudfront_origin_access_identity.react.cloudfront_access_identity_path } - custom_header { - name = "X-CloudFront-Auth" - value = var.cloudfront_auth_token - } } ... }.github/workflows/deploy_to_aws.yml (1)
165-186: Move Static Assets Step – S3 Sync and CloudFront Invalidation
This step sets up the necessary AWS environment variables and then loops through CloudFront distributions to match the one corresponding to your defined domain. The error handling (exiting if no matching distribution is found) is a good safeguard. Next, theaws s3 synccommand is invoked without the problematic--acl public-readflag, and a CloudFront invalidation is issued.A couple of suggestions:
• Although your variables (likeCLOUDFRONT_DOMAIN) are unlikely to contain spaces, consider quoting shell variables when possible for extra robustness.
• Invalidation of all paths ("/*") is sometimes considered heavy; if feasible, you might narrow this down to only the paths you expect to change (for instance"/index.html"and"/static/*").Example diff for more granular invalidation (optional):
- aws cloudfront create-invalidation --distribution-id $CLOUDFRONT_DISTRIBUTION_ID --paths "/*" + aws cloudfront create-invalidation --distribution-id "$CLOUDFRONT_DISTRIBUTION_ID" --paths "/index.html" "/static/*"Overall, the logic and AWS CLI usage in this step appear correct.
doc/release/RELEASE-NOTES.md (2)
6-13: New Release Section for 1.30.0 – Introduction and Header
A new release section for version 1.30.0 has been added with subsections covering Introduction, Database changes, Migrations, etc. The introductory details (product name and release date) are clear. Please note that markdown lint warnings have been flagged (e.g. trailing punctuation in headings on lines 7 and 36).Suggested change (if desired):
-## Release 1.30.0: +## Release 1.30.0This minor change will satisfy lint rules regarding trailing punctuation.
17-17: Markdown Lint Note – Heading Punctuation
Static analysis suggests that trailing punctuation in headings should be removed (MD026). Please consider removing any trailing punctuation (e.g. colons at the end of headings) if you wish to comply with markdown standards.Also applies to: 36-36
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
17-17: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/deploy_to_aws.yml(3 hunks)deployment/terraform/cdn.tf(4 hunks)doc/release/RELEASE-NOTES.md(1 hunks)
🧰 Additional context used
📓 Learnings (2)
.github/workflows/deploy_to_aws.yml (1)
Learnt from: roninzp
PR: opensupplyhub/open-supply-hub#449
File: .github/workflows/deploy_to_aws.yml:165-180
Timestamp: 2025-01-21T13:37:17.243Z
Learning: In the Open Supply Hub project, using the `--acl public-read` flag with the `aws s3 sync` command breaks the synchronization process. The flag should be omitted for proper functionality.
deployment/terraform/cdn.tf (3)
Learnt from: roninzp
PR: opensupplyhub/open-supply-hub#449
File: deployment/terraform/cdn.tf:5-8
Timestamp: 2025-01-27T11:38:22.010Z
Learning: S3 bucket public access blocks are deliberately omitted in the Open Supply Hub infrastructure to allow necessary access patterns through CloudFront Origin Access Identity.
Learnt from: roninzp
PR: opensupplyhub/open-supply-hub#449
File: deployment/terraform/cdn.tf:154-154
Timestamp: 2025-01-27T11:37:00.673Z
Learning: Compression is not needed for S3-hosted static assets in the Open Supply Hub project.
Learnt from: roninzp
PR: opensupplyhub/open-supply-hub#449
File: deployment/terraform/cdn.tf:0-0
Timestamp: 2025-01-20T12:53:25.057Z
Learning: In the Open Supply Hub project, S3 bucket versioning is intentionally disabled as old versions of static files don't need to be retained. The files can be rebuilt and redeployed from source if needed.
🪛 markdownlint-cli2 (0.17.2)
doc/release/RELEASE-NOTES.md
17-17: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
36-36: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-flake8-linter
- GitHub Check: run-fe-code-quality
- GitHub Check: run-eslint-linter-and-prettier-formatter
- GitHub Check: run-django-code-quality
- GitHub Check: run-dd-code-quality
- GitHub Check: run-countries-code-quality
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: get-base-branch-fe-cov
- GitHub Check: get-base-branch-django-cov
- GitHub Check: get-base-branch-contricleaner-cov
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: get-base-branch-dd-cov
🔇 Additional comments (19)
deployment/terraform/cdn.tf (11)
1-3: Local variable definition – Frontend bucket naming
The locals block is cleanly defining the bucket name by removing spaces and lowercasing both the project and environment. This is clear and consistent with naming conventions.
5-12: S3 Bucket Resource Configuration
The S3 bucket resourceaws_s3_bucket.reactis properly configured with the bucket name, force_destroy enabled, and appropriate tagging. Ensure that the tag “Name” is sufficient for your tagging strategy.
14-24: Server-Side Encryption Configuration
The encryption configuration correctly applies AES256. The rule and settings look correct; no additional parameters seem necessary.
26-32: Bucket Versioning
Versioning is set to “Disabled” as intended per project learnings. Verify that this behavior is deliberate and that you won’t need historical versions for rollback or recovery.
34-41: Bucket Ownership Controls
The ownership control configuration withBucketOwnerEnforcedmeets security best practices.
42-81: IAM Policy Document for S3 Bucket Policy
The policy document is well constructed with two statements: one denying insecure transport and one allowing CloudFront access. The use of conditions and principals is appropriate.
88-90: CloudFront Origin Access Identity (OAI)
The OAI resource is defined correctly with a comment based on the frontend bucket name.
92-114: CloudFront Distribution – First Origin (ALB)
The firstoriginblock targeting your ALB (using a custom origin config) is standard. Ensure that the domain name (origin.${local.domain_name}) is correctly defined in the overall configuration.
138-157: Default Cache Behavior
The default cache behavior is streamlined for S3 content with only GET and HEAD allowed. Compression is disabled in line 152, which follows your project’s decision that S3-hosted static assets do not need compression. The TTL values (min_ttl 0, default_ttl 0, max_ttl 300) also appear intentionally set.
159-508: Ordered Cache Behaviors & Additional Settings
All ordered cache behaviors (for paths such as “tile/”, “api/”, etc.) are defined with careful distinctions in allowed and cached methods, forwarded values (including headers and cookies), compression settings, viewer protocol policies, and TTLs. The logging configuration (lines 642-646) references an external logs bucket (aws_s3_bucket.logs) – please verify that this dependency is intended and that the logs bucket is indeed defined elsewhere in your codebase. The custom error responses (lines 660-672) correctly handle 403 and 404 errors by redirecting to/index.htmlfor SPA routing.
674-678: CloudFront Distribution Tags
The distribution tags are set using project and environment variables. This is consistent with your tagging strategy..github/workflows/deploy_to_aws.yml (3)
125-130: Workflow – Job Setup for React App Deployment
The new jobbuild_and_push_react_appis inserted after the Terraform apply job. The environment and job conditions are clearly defined. The steps for checking out the repository, setting up Node.js, caching dependencies, and building the React static assets are standard and well implemented.
160-164: Project Name Processing
The step with the IDprojectuses a minimal dependency action to transform the project variable. This is acceptable given your use case; just verify that the variablevars.PROJECTis defined in your workflow variables.
187-246: Docker Image Deployment Job
The subsequent jobbuild_and_push_docker_imagefollows immediately after React-related tasks and builds Docker images for multiple services. The use of GitHub Actions for ECR login, image build, and push steps is standard and properly configured.🧰 Tools
🪛 actionlint (1.7.4)
208-208: the runner of "aws-actions/configure-aws-credentials@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
225-225: the runner of "docker/build-push-action@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
233-233: the runner of "docker/build-push-action@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
241-241: the runner of "docker/build-push-action@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
doc/release/RELEASE-NOTES.md (5)
10-13: Introduction Section for 1.30.0
The “Introduction” subsection clearly states the product name and the release date. It meets the requirements.
14-22: Database Changes, Migrations, & Schema Changes Subsections
Placeholders for database changes, migrations, and schema changes are provided. These are appropriate if more detailed notes are expected later.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
17-17: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
23-28: Code/API Changes – Splitting FE and BE
The “Code/API changes” subsection explicitly describes the architectural change where the Django container has been split into two components (FE and BE). This is a key point of the release. The description is clear and links to the relevant ticket (OSDEV-899).
29-38: Architecture/Environment, Bugfix, What's New, and Release Instructions Subsections
These subsections have been added to provide comprehensive notes on changes. The content is well structured and clearly explains what is new for version 1.30.0. Remember to include any additional release instructions or important pre-/post-deployment steps in this section.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
36-36: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
39-44: Existing Release Sections
The previous release sections (such as 1.29.0) are preserved as expected. This helps maintain historical context.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
41-41: Multiple headings with the same content
null(MD024, no-duplicate-heading)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
doc/release/RELEASE-NOTES.md (8)
6-7: Placeholder Instruction NoticeThe line "Use the format below to document information about the new release." appears to be an instructional placeholder. Please remove or replace this text with the finalized release note details before publishing.
14-16: Database Changes Section PlaceholderThe "### Database changes" section currently includes placeholder text (“* Describe high-level database changes.”). Please update this section with the actual database changes or remove the placeholder text if no changes apply.
17-19: Migrations Section Heading: Trailing Punctuation & PlaceholderThe heading "#### Migrations:" ends with a colon, which conflicts with markdown style guidelines (MD026). Please change it to "#### Migrations" (without the colon). Also, update the placeholder (“* Describe migrations here.”) with detailed migration information.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
17-17: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
20-22: Schema Changes Section PlaceholderThe "#### Schema changes" section still has placeholder content (“* Describe schema changes here.”). Consider providing concrete details about any schema modifications or removing the placeholder if not applicable.
23-25: Code/API Changes Section PlaceholderThis section contains placeholder text (“* Describe code/API changes here.”). Please replace it with a summary of the actual code or API changes introduced in this release.
30-32: Bugfix Section PlaceholderThe "### Bugfix" section currently displays placeholder text (“* Describe bugfix here.”). Please update this section with a summary of any bug fixes incorporated in this release or remove it if there are none.
33-35: What's New Section PlaceholderThe "### What's new" section shows placeholder text (“* Describe what's new here. The changes that can impact user experience should be listed in this section.”). Update this with a concise list of user-impacting changes or remove the placeholder if not applicable.
36-38: Release Instructions Heading: Trailing Punctuation & PlaceholderThe "### Release instructions:" heading ends with a colon, which goes against markdownlint guidelines. Please modify it to "### Release instructions" (without the colon). Additionally, the section still contains placeholder text (“* Provide release instructions here.”); be sure to replace it with the actual post-deployment steps or remove it if not needed.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
36-36: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
doc/release/RELEASE-NOTES.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
doc/release/RELEASE-NOTES.md
17-17: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
36-36: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: run-flake8-linter
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-dd-code-quality
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: run-django-code-quality
- GitHub Check: run-fe-code-quality
- GitHub Check: run-countries-code-quality
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: get-base-branch-contricleaner-cov
- GitHub Check: get-base-branch-fe-cov
- GitHub Check: get-base-branch-django-cov
🔇 Additional comments (2)
doc/release/RELEASE-NOTES.md (2)
8-13: Release Header and Introduction DetailsThe new release header ("## Release 1.30.0") along with the introduction section—including the product name and release date—are well formatted and provide clear context for this new release.
26-28: Architecture/Environment Changes DetailsThe "### Architecture/Environment changes" section now includes a detailed note referencing [OSDEV-899] about splitting the Django container into FE (React) and BE (Django) components, along with the implications for CDN routing. This level of detail is clear and helpful.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/deploy_to_aws.yml (1)
165-186: Static Assets Move Step ("Move static")
This step is central to the new workflow. A couple of points:
• The script iterates over all CloudFront distributions and checks if theAliases.Itemsexactly equal the providedCLOUDFRONT_DOMAIN. This approach works as long as the actual values never include unexpected spaces or special characters – which, as discussed earlier with your team, is acceptable.
• The error handling (lines 180–183) properly exits if no matching distribution is found.
• The S3 sync command now omits the problematic--acl public-readflag, which aligns with your PR objective.
• Finally, the CloudFront invalidation command is still set to invalidate all paths ("/*"). While this is acceptable in many cases, you might later consider whether narrowing the invalidation (for example, to only changed assets such as/static/*and/index.html) could improve performance if your asset set grows large.doc/release/RELEASE-NOTES.md (2)
15-17: Heading Trailing Punctuation – Migrations
The heading “#### Migrations:” contains a trailing colon. According to MD026 (“no-trailing-punctuation”) this should be removed for consistency.
Suggested diff:-#### Migrations: +#### Migrations🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
15-15: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
60-62: Heading Trailing Punctuation – Release Instructions
Similarly, the heading “### Release instructions:” also ends with a colon. Removing the trailing punctuation would bring it in line with markdown linting guidelines.
Suggested diff:-### Release instructions: +### Release instructions🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
60-60: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/deploy_to_aws.yml(3 hunks)doc/release/RELEASE-NOTES.md(1 hunks)
🧰 Additional context used
📓 Learnings (1)
.github/workflows/deploy_to_aws.yml (1)
Learnt from: roninzp
PR: opensupplyhub/open-supply-hub#449
File: .github/workflows/deploy_to_aws.yml:165-180
Timestamp: 2025-01-21T13:37:17.243Z
Learning: In the Open Supply Hub project, using the `--acl public-read` flag with the `aws s3 sync` command breaks the synchronization process. The flag should be omitted for proper functionality.
🪛 markdownlint-cli2 (0.17.2)
doc/release/RELEASE-NOTES.md
15-15: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
60-60: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-flake8-linter
- GitHub Check: run-fe-code-quality
- GitHub Check: run-django-code-quality
- GitHub Check: run-dd-code-quality
- GitHub Check: run-countries-code-quality
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: get-base-branch-fe-cov
- GitHub Check: get-base-branch-contricleaner-cov
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: get-base-branch-django-cov
🔇 Additional comments (4)
.github/workflows/deploy_to_aws.yml (2)
125-130: New React Deployment Job Definition
The addition of the new jobbuild_and_push_react_appis well structured. It correctly depends on the precedingapplyjob and uses a similar environment expression as the other jobs. Please double‑check that the long conditional forenvironment:exactly matches your naming and deployment conventions.
160-164: Project Name Transformation Step
The new step that sets the project name (via theEntepotenz/change-string-case-action-min-dependenciesaction) looks good. It ensures that you have a lowercase version available for dynamically building the S3 bucket name.doc/release/RELEASE-NOTES.md (2)
6-11: New Release Section for Version 1.30.0
The new release notes for version 1.30.0 are comprehensive and follow the established structure. They clearly detail the separation of the React frontend (now deployed to S3/CloudFront) and Django backend, along with the associated infrastructure changes.
24-35: Architecture/Environment Changes Clarity
In the “### Architecture/Environment changes” section, the bullet point for [OSDEV-899] clearly explains the split between FE (React) and BE (Django), including the CDN handling. This is good documentation for the change. Just ensure that any additional details (if applicable later) about S3 naming conventions are consistent with your workflow.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
doc/release/RELEASE-NOTES.md (2)
6-61: New Release 1.30.0 Section – Confirm Feature Details and Placeholders
The new release section for version 1.30.0 is well structured. It clearly describes the architectural change splitting the Django container into two components with the frontend (React) now served via a CDN (CloudFront) and the backend (Django) handling API requests. Please verify that the description explicitly emphasizes that the React application is now hosted in the S3 bucket—as noted in the PR objectives—and consider adding more details (if needed) regarding the S3 deployment specifics.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
15-15: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
59-59: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
59-61: Formatting Suggestion: Remove Trailing Punctuation in Headings
Markdownlint suggests avoiding trailing punctuation in headings (MD026). For example, the heading “### Release instructions:” on line 59 would be better formatted without the colon. Consider updating it as follows:-### Release instructions: +### Release instructionsThe same suggestion applies to any other headings ending with a colon (such as “#### Migrations:” on line 15).
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
59-59: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
doc/release/RELEASE-NOTES.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
doc/release/RELEASE-NOTES.md
15-15: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
59-59: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-flake8-linter
- GitHub Check: run-fe-code-quality
- GitHub Check: run-eslint-linter-and-prettier-formatter
- GitHub Check: run-dd-code-quality
- GitHub Check: run-django-code-quality
- GitHub Check: run-countries-code-quality
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: get-base-branch-fe-cov
- GitHub Check: get-base-branch-django-cov
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: get-base-branch-contricleaner-cov
🔇 Additional comments (1)
doc/release/RELEASE-NOTES.md (1)
15-17: Placeholder Headings – Verify Completion Before Release
The headings under Database changes, Migrations, and Schema changes currently use placeholders (e.g. "Describe high-level database changes.", "Describe migrations here."). Please ensure that these are either filled in or confirmed as intentionally left as placeholders if no database changes occur in this release.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
15-15: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
VadimKovalenkoSNF
left a comment
VadimKovalenkoSNF
left a comment
There was a problem hiding this comment.
Request to change major release version.
cc: @vladsha-dev
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
doc/release/RELEASE-NOTES.md (6)
12-14: Database Changes Placeholder Present
A placeholder for database changes is provided. If no database modifications are needed, consider removing the placeholder text or updating it with a brief note (e.g., “No database changes in this release”).
15-17: Trailing Punctuation in 'Migrations' Heading
The heading “#### Migrations:” ends with a colon, which violates markdown style (MD026, no-trailing-punctuation).
Consider updating it as follows:-#### Migrations: +#### Migrations🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
15-15: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
21-23: Code/API Changes Section Placeholder
The “### Code/API changes” section is provided with placeholder text. Once the specifics are finalized, please update it with details to avoid ambiguity.
53-55: Bugfix Section Placeholder
A placeholder is provided for bugfixes. If no bug fixes were delivered in this release, consider either removing the placeholder or replacing it with “No bug fixes in this release” to reduce confusion.
56-58: 'What's New' Section Placeholder
The “### What's new” section currently holds placeholder text. For clarity, update this section with new user-impacting changes or features as soon as the details are available.
59-61: Trailing Punctuation in 'Release instructions' Heading
The “### Release instructions:” heading includes a trailing colon, which again violates markdown style guidelines (MD026). Please remove the colon.
A proposed change:-### Release instructions: +### Release instructions🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
59-59: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
doc/release/RELEASE-NOTES.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
doc/release/RELEASE-NOTES.md
15-15: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
59-59: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-flake8-linter
- GitHub Check: run-fe-code-quality
- GitHub Check: run-eslint-linter-and-prettier-formatter
- GitHub Check: run-django-code-quality
- GitHub Check: run-dd-code-quality
- GitHub Check: run-countries-code-quality
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: get-base-branch-fe-cov
- GitHub Check: get-base-branch-django-cov
- GitHub Check: get-base-branch-contricleaner-cov
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: get-base-branch-dd-cov
🔇 Additional comments (6)
doc/release/RELEASE-NOTES.md (6)
6-7: New Release Section for Version 2.0.0 Added
A new release section for version 2.0.0 has been introduced with all the standard subsections (Introduction, Database changes, Migrations, etc.). This aligns with the PR objectives outlining the architectural changes (e.g., the separation of the Django container).
8-11: Introduction Section Looks Clear
The introduction clearly lists the product name and the release date. Ensure that both values remain accurate and aligned with your deployment plans.
18-20: Schema Changes Section Looks Good
The “#### Schema changes” section is present with a placeholder for description. Ensure that, if schema modifications occur, they are detailed here.
24-27: Architecture/Environment Changes Documented Properly
The architecture/environment changes section documents the key change (splitting the Django container) with reference to [OSDEV-899]. The level of detail appears sufficient; please double-check that all endpoints are current with the intended routing changes.
27-50: Comprehensive List of Redirected Endpoints
The bullet list of endpoints to be redirected to the Django container is extensive. Verify that this list is up-to-date and that no essential routes have been omitted. A periodic review of these endpoints may help maintain accuracy as the application evolves.
51-52: Redirection for All Other Traffic
The note specifying that all other traffic will be redirected to the React application is clear and concise.
3 participants
With this task, we split the Django container into two components: FE (React) and BE (Django). Requests to the frontend (React) will be processed by the CDN (CloudFront), while requests to the API will be redirected to the Django container. This approach will allow for more efficient use of ECS cluster computing resources and improve frontend performance.
The following endpoints will be redirected to the Django container:
All other traffic will be redirected to the React application.