We are extending the ConfigFormBase by SchedulerAdminForm and yet using t(). Shouldn't be Oops version of this ?

CommentFileSizeAuthor
#9 2612606-9.patch6.11 KBpfrenssen
#8 2612606-8.patch6.05 KBjoshi.rohit100
#2 2612606-2.patch5.86 KBjoshi.rohit100
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joshi.rohit100 created an issue. See original summary.

joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
CreditAttribution: joshi.rohit100 commented
Status: Active » Needs review
FileSize
2612606-2.patch5.86 KB

Status: Needs review » Needs work

The last submitted patch, 2: 2612606-2.patch, failed testing.

jonathan1055’s picture

jonathan1055 CreditAttribution: jonathan1055 commented

This patch is going to clash with the work in #2612326: Change ! placeholders in formatted messages. What D8 version are you running with? The ! placeholder is no longer valid in normal calls to t( ). Just wondering how we want to manage this so we do not step on each other toes.

Jonathan

joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
CreditAttribution: joshi.rohit100 commented
Status: Needs work » Postponed

this is for 8.x-1.x-dev version. For this, we can postponed this until #2612326: Change ! placeholders in formatted messages lands

legovaer’s picture

legovaer
he/him
Primary language Dutch
Location Duffel
CreditAttribution: legovaer as a volunteer and at Capgemini commented
Status: Postponed » Active

Re-opening because #2612326: Change ! placeholders in formatted messages has been fixed

joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
CreditAttribution: joshi.rohit100 commented
Assigned: Unassigned » joshi.rohit100
joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
CreditAttribution: joshi.rohit100 commented
Status: Active » Needs review
FileSize
2612606-8.patch6.05 KB
pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen commented
FileSize
2612606-9.patch6.11 KB

Rerolled patch, it didn't apply any more. Will review now.

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen commented
Assigned: joshi.rohit100 » Unassigned
Status: Needs review » Fixed

Looking great, thanks for the find & fix!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

The last submitted patch, 8: 2612606-8.patch, failed testing.

Status: Closed (fixed) » Needs work

The last submitted patch, 9: 2612606-9.patch, failed testing.

jonathan1055’s picture

jonathan1055 CreditAttribution: jonathan1055 as a volunteer commented
Status: Needs work » Closed (fixed)

Ignore the failed patch above. It was queued but not run until the 8.x committed codebase passed all tests. That happened with my commit a few minutes ago, and then the untested patches have suddenly come to life and been run. This issue is already fixed.