Replacing t(' with $this->t (Group module [d8])

CommentFileSizeAuthor
#9 group-2826874-9.patch7.74 KBlobsterr
#7 after.png78.19 KBlomasr
#6 group-2826874-6.patch7.77 KBkristiaanvandeneynde
#2 2826874-2.patch18.02 KBlomasr

Comments

lomasr created an issue. See original summary.

lomasr’s picture

lomasr commented
StatusFileSize
new 2826874-2.patch18.02 KB

Adding a patch .Please review.

lomasr’s picture

lomasr commented
Status: Active » Needs review

Status: Needs review » Needs work

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

lomasr’s picture

lomasr commented
Version: 8.x-1.x-dev » 8.x-1.0-beta4
kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented
Status: Needs work » Needs review
StatusFileSize
new group-2826874-6.patch7.77 KB
  • GroupNodeDeriver does not implement StringTranslationTrait so cannot use $this->t().
  • Same for all of the cache contexts
  • Same for all of the entity classes (Group, GroupContent, etc.)
  • On top of that, Group::baseFieldDefinitions(), GroupContent::baseFieldDefinitions(), etc. are static so they can't even use $this

GroupTypeListBuilder, GroupRoleDeleteForm, GroupRoleForm, GroupPermissionsRoleSpecificForm and GroupContentViewsData did have the trait (all forms do) and could thus use it.

Attached is a smaller patch fixing the places that were valid. If we want to use $this->t() in other places, we need to look into using the trait there and whether it is useful to actually do so.

lomasr’s picture

lomasr commented
StatusFileSize
new after.png78.19 KB

Thanks for the clarification. I applied the patch in #6 It didn't worked for me . Please see the after.png.

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented

Yeah it needs a reroll.

lobsterr’s picture

lobsterr commented
StatusFileSize
new group-2826874-9.patch7.74 KB

I've added a reroll

i-trokhanenko’s picture

i-trokhanenko
Location Lutsk 🇺🇦
commented
Version: 8.x-1.0-beta4 » 8.x-1.x-dev
Status: Needs review » Reviewed & tested by the community

Patch #9 applied correctly to 8.x-1.x-dev. Changes looks good for me.
+1 RTBC

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented
Status: Reviewed & tested by the community » Fixed

Looks good, thanks!

Status: Fixed » Closed (fixed)

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

lomasr’s picture

lomasr commented