Follow up to #2157691: Move some helper methods in errors.inc to an Error utility class

CommentFileSizeAuthor
#6 2170519-testbase-doc-6.patch1002 byteslongwave
#1 2170519-testbase-doc-1.patch930 byteskim.pepper

Comments

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented
Status: Active » Needs review
StatusFileSize
new 2170519-testbase-doc-1.patch930 bytes

Comment fixes.

damiankloip’s picture

damiankloip commented
Status: Needs review » Reviewed & tested by the community
jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

You could fix the comment wrapping -- that comment does not have to be so many lines?

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Needs work
kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented
You could fix the comment wrapping -- that comment does not have to be so many lines?

How do you propose to fix the wrapping? Break the namespace up?

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Needs review
StatusFileSize
new 2170519-testbase-doc-6.patch1002 bytes

We can reword the comment as per the attachment to make it not look quite so strange in code.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Oh sorry, my bad -- you are right that the comment wouldn't all fit on one line. I must have been halucinating or had my editor/terminal window mis-sized there.

I like the wording in #6 better though -- active rather than passive voice is always preferable. Thanks!

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

There is an "avoid commit conflicts" patch on #1862202: Objectify the language system that touches this same file, and although they don't seem to conflict, I'm being extra cautious lately and will wait to commit this until that one lands. And then I'll probably have to wait about a week during our "only commit major/critical patches the week before the next alpha" break. Sorry...

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

The other patch landed and we're out of "critical/major" week, so I committed this to 8.x. Thanks again!

Status: Fixed » Closed (fixed)

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