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

fix: DAH-3187 update FCFS how to apply link translations #2513

Merged
merged 4 commits into from
Jan 29, 2025

Conversation

cliu02
Copy link
Collaborator

@cliu02 cliu02 commented Jan 24, 2025

Description

Update translations for FCFS how to apply link.

Jira ticket

https://sfgovdt.jira.com/browse/DAH-3187

Before requesting eng review

Version Control

  • branch name begins with angular if it contains updates to Angular code
  • branch name contains the Jira ticket number
  • PR name follows type: TICKET-NUMBER Description format, e.g. feat: DAH-123 New Feature. If the PR is urgent and does not need a ticket then use the format urgent: Description

Code quality

  • the set of changes is small
  • all automated code checks pass (linting, tests, coverage, etc.)
  • code irrelevant to the ticket is not modified e.g. changing indentation due to automated formatting
  • if the code changes the UI, it matches the UI design exactly

Review instructions

  • instructions specify which environment(s) it applies to
  • instructions work for PA testers
  • instructions have already been performed at least once

Request eng review

  • PR has needs review label
  • Use Housing Eng group to automatically assign reviewers, and/or assign specific engineers
  • If time sensitive, notify engineers in Slack

Before merging

Request product acceptance testing

  • Code change is behind a feature flag
  • If code change is not behind a feature flag, it has been PA tested in the review environment (use needs product acceptance label to indicate that the PR is waiting for PA testing)

@cliu02 cliu02 temporarily deployed to dahlia-webapp-pr-2513 January 24, 2025 00:49 Inactive
@cliu02 cliu02 temporarily deployed to dahlia-webapp-pr-2513 January 28, 2025 23:49 Inactive
@cliu02 cliu02 marked this pull request as ready for review January 28, 2025 23:49
@cliu02 cliu02 added the needs review Pull request needs review label Jan 28, 2025
@cliu02 cliu02 requested review from a team, chadbrokaw and jimlin-sfgov and removed request for a team January 28, 2025 23:52
@jimlin-sfgov
Copy link
Collaborator

@cliu02 This PR modifies non-english translation keys. Did you first make the changes in Phrase, and then do a phrasePull per our guide?

Copy link
Collaborator

@cade-exygy cade-exygy left a comment

Choose a reason for hiding this comment

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

Looks Good!

@cliu02
Copy link
Collaborator Author

cliu02 commented Jan 29, 2025

@jimlin-sfgov not yet, I can do that! My understanding from our team eng sync is that we decided to remove the translations checkbox for the PR review template and work with Bridget to update Phrase before sending each round instead.

@jimlin-sfgov
Copy link
Collaborator

jimlin-sfgov commented Jan 29, 2025

we decided to remove the translations checkbox for the PR review template and work with Bridget to update Phrase before sending each round instead

@cliu02 I guess we didn't make it clear enough during eng. sync. For the "standard" workflow of adding new translations, engineers do not need to interact with Phrase. For other translation workflows, there are additional steps that needed to be followed in the guide. Especially for this PR's case, changes must be made in Phrase first, otherwise it becomes problematic for whoever manages the next batch of human translations.

@cliu02 cliu02 temporarily deployed to dahlia-webapp-pr-2513 January 29, 2025 19:32 Inactive
@cliu02
Copy link
Collaborator Author

cliu02 commented Jan 29, 2025

@jimlin-sfgov got it sorry about that, @chadbrokaw has helped update these strings in Phrase!

Copy link
Collaborator

@jimlin-sfgov jimlin-sfgov left a comment

Choose a reason for hiding this comment

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

LGTM

@cliu02 cliu02 merged commit 2204905 into main Jan 29, 2025
20 checks passed
@cliu02 cliu02 deleted the DAH-3187-fcfs-link-translations branch January 29, 2025 20:57
cade-exygy pushed a commit that referenced this pull request Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review Pull request needs review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants