Skip to content
New issue

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

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

Already on GitHub? # to your account

Chore: change SacRT copy to say "bus" instead of "light rail" #2507

Merged
merged 2 commits into from
Nov 5, 2024

Conversation

angela-tran
Copy link
Member

@angela-tran angela-tran commented Nov 5, 2024

Part of #2505

There were 3 places I found that mentioned "light rail":

  • "Get a reduced fare on SacRT light rail when you tap to ride" - agency index page
  • "Cal-ITP doesn’t save any of your information. All SacRT transit benefits reduce fares by 50% for light rail." - eligibility index
  • "You were not charged anything today. When boarding SacRT light rail, tap this card and you will be
    charged a reduced fare. You will need to re-enroll if you choose to change the card you use to pay for transit service." - enrollment success

For the Spanish translations, I did my best to map "bus" in for SacRT based on the copy for SBMTD. @lalver1 Maybe you can check my work to see if it makes sense to a Spanish speaker?

SBMTD entries in es/LC_MESSAGES/django.po are right below SacRT's entries, so you can compare them pretty easily.

@angela-tran angela-tran self-assigned this Nov 5, 2024
@angela-tran angela-tran requested a review from a team as a code owner November 5, 2024 20:56
@github-actions github-actions bot added front-end HTML/CSS/JavaScript and Django templates deployment-dev [auto] Changes that will trigger a deploy if merged to dev i18n Copy: Language files or Django i18n framework and removed front-end HTML/CSS/JavaScript and Django templates deployment-dev [auto] Changes that will trigger a deploy if merged to dev labels Nov 5, 2024
Copy link

github-actions bot commented Nov 5, 2024

Coverage report

This PR does not seem to contain any modification to coverable code.

@angela-tran
Copy link
Member Author

angela-tran commented Nov 5, 2024

Hmm, I'm not sure why the Django migration+messages check is failing... I did run ./bin/makemessages.sh and commit the changes. 🙁

edit: also when I run the commands that the workflow is running, I get the output that should make the workflow pass. ☹️

./bin/makemessages.sh
git add benefits
git diff --cached --shortstat

gives me

 2 files changed, 2 insertions(+), 2 deletions(-)

@lalver1
Copy link
Member

lalver1 commented Nov 5, 2024

The translations look great @angela-tran!

@angela-tran
Copy link
Member Author

angela-tran commented Nov 5, 2024

Hmm, I'm not sure why the Django migration+messages check is failing... I did run ./bin/makemessages.sh and commit the changes. 🙁
...

~~never mind~~ I think this is due to how GitHub pull requests are technically running on a hidden branch that compares the base and target branches.

I am able to reproduce the failing check locally with these steps:

  • Create a script at the project root-level containing what the check runs, call it check.sh:
./bin/makemessages.sh

set -x # show commands

git add benefits

# message files are up-to-date if the only differences are from the updated timestamp
if echo $(git diff --cached --shortstat) | grep -q '2 files changed, 2 insertions(+), 2 deletions(-)';
then
  exit 0;
else
  exit 1;
fi
  • Fetch the hidden branch into your repo as a local branch named troubleshoot-makemessages (or whatever you want):
    • git fetch origin pull/2507/head:troubleshoot-makemessages
  • Check it out
    • git checkout troubleshoot-makemessages
  • Run the script
    • ./check.sh

My output

calitp@7b79b09cfce6:/calitp/app$ ./check.sh 
processing locale en
processing locale es
++ git add benefits
++ grep -q '2 files changed, 2 insertions(+), 2 deletions(-)'
+++ git diff --cached --shortstat
++ echo 2 files changed, 7 'insertions(+),' 5 'deletions(-)'
++ exit 1

These sources confirm that there is a "hidden branch" or whatever it is

their plan is to roll out the new system on buses first.

for the Spanish translations, I did my best to map "bus" in based on
the copy for SBMTD.
@angela-tran angela-tran force-pushed the chore/sacrt-copy-update branch from ebeb121 to 085391e Compare November 5, 2024 21:40
@angela-tran
Copy link
Member Author

angela-tran commented Nov 5, 2024

Ok, never mind. I'm not sure what state I was in before, but I just re-checked out chore/sacrt-copy-update, ran ./bin/makemessages.sh, and there were changes! Minor formatting things that don't actually change the copy. 🙄

Committed those, and the check should pass now. I don't think it had anything to do with the special /pull/2507 ref.

Copy link
Member

@lalver1 lalver1 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! 👍

@angela-tran angela-tran merged commit 2e2e4f4 into main Nov 5, 2024
10 checks passed
@angela-tran angela-tran deleted the chore/sacrt-copy-update branch November 5, 2024 22:37
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
i18n Copy: Language files or Django i18n framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants