Problem/Motivation

From #1315886: Clean up API docs for includes directory, files starting with A-C. archiver.inc has the following minor code style issues:

  • Double space after a period in constructor documentation.
  • Missing period in the documentation for ArchiverInterafce::extract().
  • Incorrect capitalization of type-hinting in ArchiverInterafce::extract(). (Array should be array.)

Proposed resolution

Correct these style issues.

Remaining tasks

Postponed for #1315886: Clean up API docs for includes directory, files starting with A-C.

User interface changes

None.

API changes

None.

CommentFileSizeAuthor
#7 clean-up-minor-code-1326482-7.patch698 bytesryan.gibson
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Status: Active » Postponed
Issue tags: -docs-cleanup-2011
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Issue tags: +Needs backport to D7

This is probably unobtrusive enough to backport.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Assigned: Unassigned » xjm
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Priority: Normal » Minor
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Assigned: xjm » Unassigned
Issue tags: +Novice

Actually, this would make a good novice issue.

Note that any patch for this issue will need to be rerolled once #1315886: Clean up API docs for includes directory, files starting with A-C is fixed, so probably best to wait until then.

ryan.gibson’s picture

ryan.gibson CreditAttribution: ryan.gibson commented
Assigned: Unassigned » ryan.gibson

I can do this one :)

ryan.gibson’s picture

ryan.gibson CreditAttribution: ryan.gibson commented
Status: Postponed » Needs review
FileSize
clean-up-minor-code-1326482-7.patch698 bytes

Okay, this is my first time creating and uploading a patch - I couldn't see a double-space after a period in the constructor documentation. If I missed it, I can fix it. Wow, core office hours really are a great way to both help and learn. Now I can see a patch and understand it much better. I thought all of the patch info was typed manually. GIT really does the hard work.

tregeagle’s picture

tregeagle CreditAttribution: tregeagle commented
Status: Needs review » Reviewed & tested by the community

Hey Ryan,
Nice one. The patch applies cleanly and it looks like all the typos are fixed.
Now I just need to find my first patch to roll...

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Not sure why the testbot didn't report back, but the tests passed.

Committed and pushed to 8.x and 7.x. Thanks! Congrats on your first core patch, Ryan! :D

jthorson’s picture

jthorson CreditAttribution: jthorson commented
Status: Fixed » Needs review
Issue tags: -Novice, -Needs backport to D7

#7: clean-up-minor-code-1326482-7.patch queued for re-testing.

Sorry ... just troubleshooting testbots.

Status: Needs review » Needs work
Issue tags: +Novice, +Needs backport to D7

The last submitted patch, clean-up-minor-code-1326482-7.patch, failed testing.

jthorson’s picture

jthorson CreditAttribution: jthorson commented
Status: Needs work » Fixed

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

Anonymous’s picture

(not verified) CreditAttribution: commented
Issue summary: View changes

Added clarification.