Skip to content

Conversation

@jamil314
Copy link
Contributor

Note

Currently, we do not run e2e tests as a check on opencrvs-countryconfig-repo PRs. Please ensure your PR doesn't break any e2e tests.

One method for doing this is to open a PR with these changes to opencrvs-farajaland as well, and see if the PR check passes there.

Description

Clearly describe what has been changed. Include relevant context or background.
Explain how the issue was fixed (if applicable) and the root cause.

Link this pull request to the GitHub issue (and optionally name the branch ocrvs-<issue #>)

Checklist

  • I have linked the correct Github issue under "Development"
  • I have tested the changes locally, and written appropriate tests
  • I have tested beyond the happy path (e.g. edge cases, failure paths)
  • I have updated the changelog with this change (if applicable)
  • I have updated the GitHub issue status accordingly

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

12 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile


- Restricted filesystem usage for journal service and file rotation strategy [#10518](https://github.com/opencrvs/opencrvs-core/issues/10518))

- Tiltfile: Improved Kubernetes support for development environment [#10672](https://github.com/opencrvs/opencrvs-core/issues/10672)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: Missing space after link number in markdown

Comment on lines +13 to +14
# FIXME: Replace ocrvs-10672 to develop after testing
git_checkout('[email protected]:opencrvs/infrastructure.git#ocrvs-10672', '../infrastructure')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: remove FIXME comment and hardcoded branch reference before merging to release. Should this be pointing to 'develop' for the v1.9.0 release, or is ocrvs-10672 the correct branch to use?

# ------ REINDEX -----
##
docker run --rm \
-v /opt/opencrvs/infrastructure/deployment:/workspace \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: The reindex script path is hardcoded to /opt/opencrvs/infrastructure/deployment. This will fail in local environments where ROOT_PATH is different (set to ../opencrvs-core/data on line 75). Consider using $ROOT_PATH/infrastructure/deployment or adding a conditional check for IS_LOCAL. Should the reindex step be skipped in local environments, or should it use a dynamic path based on ROOT_PATH?

-e 'AUTH_URL=http://auth:4040/' \
-e 'EVENTS_URL=http://gateway:7070/events' \
alpine \
sh -c 'apk add --no-cache curl jq && sh reindex.sh'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Missing newline at end of file

@Zangetsu101 Zangetsu101 merged commit 931d10a into release-v1.9.0 Oct 23, 2025
5 checks passed
@Zangetsu101 Zangetsu101 deleted the sync-release-v1.9.0 branch October 23, 2025 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants