Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: cond routingv1.1 #8031

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from
Open

feat: cond routingv1.1 #8031

wants to merge 11 commits into from

Conversation

kevin9foong
Copy link
Contributor

Problem

General changes to improve experience for conditional routing before GA.

Closes FRM-1927

Solution

Breaking Changes

No - this PR is backwards compatible

Tests

@kevin9foong kevin9foong requested a review from KenLSM January 7, 2025 06:40
@kevin9foong kevin9foong self-assigned this Jan 7, 2025
Copy link

linear bot commented Jan 7, 2025

@datadog-opengovsg
Copy link

datadog-opengovsg bot commented Jan 7, 2025

Datadog Report

Branch report: feat/cond-routingv1.1
Commit report: d6fa6a3
Test service: formsg

✅ 0 Failed, 1746 Passed, 0 Skipped, 3m 54.92s Total duration (2m 59.78s time saved)

@kevin9foong kevin9foong force-pushed the feat/cond-routingv1.1 branch from bdb6c8f to 676b4f4 Compare January 7, 2025 07:15
@kevin9foong kevin9foong force-pushed the feat/cond-routingv1.1 branch from 676b4f4 to d6fa6a3 Compare January 8, 2025 06:41
@kevin9foong
Copy link
Contributor Author

need to re-eval / test if using , for delimiter in nric is acceptable.

@kevin9foong
Copy link
Contributor Author

need to re-eval / test if using , for delimiter in nric is acceptable.

I've verified with Kenneth yesterday and tested. works as expected. No issues.

PR ready for review!

Copy link
Contributor

Choose a reason for hiding this comment

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

This file is quite huge, standing at 9MB. Did a brief check on our gifs in the repo, they aren't so huge (~300kb).

@alicia-ogp you have a smaller version of the gif? What something along the lines of 1000width instead of 1600width

Copy link
Contributor

@KenLSM KenLSM left a comment

Choose a reason for hiding this comment

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

LGTM except for the large gif size, expecting around <1mb.

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.

2 participants