We want to make the "Database" title on the fieldset specific about what database we're talking about, and describe what's going to be done with it. Change the title to "Source Database" (or whatever) and add a description.

CommentFileSizeAuthor
#8 2281767-8.patch550 bytestadityar
#6 2281767-6.patch549 bytestadityar
#4 2281767-4.patch567 bytesPalashvijay4O
Members fund testing for the Drupal project. Drupal Association Learn more

Comments

mikeryan’s picture

Issue tags: +Novice
hussainweb’s picture

Version: » 8.x-1.x-dev
Status: Active » Postponed
mikeryan’s picture

Status: Postponed » Active
Palashvijay4O’s picture

Status: Active » Needs review
FileSize
567 bytes

Submitting a patch , please suggest changes if you want any .

hass’s picture

Status: Needs review » Needs work

We don't use string concatenation inside t() and database in title need to be written lowercase.

tadityar’s picture

Status: Needs work » Needs review
FileSize
549 bytes

Corrected the title and description.

hass’s picture

Status: Needs review » Reviewed & tested by the community

That looks good to me.

tadityar’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
550 bytes

Forgot the comma at the end of description.

Palashvijay4O’s picture

@hass I guess string concatenation in t() is not a problem when we are not concatinating the variables . What do you think ?

hass’s picture

What do you mean?

#8 is correct. There is no other alternative that is correct. Translatable strings are not allowed to have string concatenation. Variables are added with placeholders. Please see i18n doc pages.

hass’s picture

Status: Needs review » Reviewed & tested by the community

Ok. Comma is correct.

hussainweb’s picture

@hass: I think Palashvijay4O was talking about his patch in #4 and your reply in #5. Essentially, if the line is too long, is string concatenation okay as long as it does not include variables?

hass’s picture

No, this is not correct as potx does not support this. Max line length of 80 is *no* limit for translatable strings!

mikeryan’s picture

Status: Reviewed & tested by the community » Fixed

Committed (with a little tweak to the description language), thanks!

Status: Fixed » Closed (fixed)

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