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

AO3-4308 Update claim email text with t's suggestion #4796

Merged
merged 3 commits into from
Apr 23, 2024

Conversation

brianjaustin
Copy link
Member

@brianjaustin brianjaustin added Priority: High - Broken on Test Merge immediately after approval Scope: i18n Only Only moves text to en.yml for i18n purposes labels Apr 22, 2024
@@ -25,7 +25,7 @@
<% end %>
</ul>

<p><%= t(".redirects") %></p>
<p><%= t(".redirects_html") %></p>
Copy link
Contributor

@Bilka2 Bilka2 Apr 22, 2024

Choose a reason for hiding this comment

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

Would be nice to have the locale keys be redirects.html and redirects.text for consistency with other mailer keys. See also #4519 (comment).

Copy link
Contributor

@Bilka2 Bilka2 Apr 22, 2024

Choose a reason for hiding this comment

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

Note: This is a nitpick. If getting this merged is very high priority, the rest of the commit looks good and this could be merged as is.

Copy link
Contributor

@Bilka2 Bilka2 left a comment

Choose a reason for hiding this comment

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

Thank you!

@brianjaustin
Copy link
Member Author

(marking as action needed again after some discussion with @sarken elsewhere)

@Bilka2
Copy link
Contributor

Bilka2 commented Apr 23, 2024

It's not clear to me whether this still applies, but in a past PR it was preferred to have a html and text version of a html safe key even when they use the same English text.

@sarken
Copy link
Collaborator

sarken commented Apr 23, 2024

(I left a note with Translation to check if they needed separate versions ~3 hours ago, but I've got to go to sleep -- I'll let you know what they say when I have a chance. If we're in a hurry, though, I'd err on the side of separate versions.)

@sarken
Copy link
Collaborator

sarken commented Apr 23, 2024

All good, thank you!

@sarken sarken merged commit e308ea9 into otwcode:master Apr 23, 2024
26 checks passed
@brianjaustin brianjaustin deleted the AO3-4308-BOT branch April 26, 2024 00:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Coder Has Actioned Review Priority: High - Broken on Test Merge immediately after approval Scope: i18n Only Only moves text to en.yml for i18n purposes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants