Part of meta-issue #1310084: [meta] API documentation cleanup sprint

CommentFileSizeAuthor
#19 clean_up_api_docs_profiles-1355362-19.patch2.83 KBbartlantz
#14 clean_up_api_docs_profiles_directory-1355362-14.patch2.86 KBbartlantz
#9 clean_up_api_docs_profiles_directory-1355362-9.patch2.87 KBbartlantz
#8 clean_up_api_docs_profiles_directory-1355362-8.patch2.87 KBbartlantz
#4 clean_up_api_docs_profiles_directory-1355362-4.patch2.87 KBbartlantz
#2 clean_up_api_docs_profiles_directory-1355362-3.patch2.87 KBbartlantz
#1 clean_up_api_docs_profiles_directory-1355362-2.patch3.06 KBbartlantz

Comments

bartlantz’s picture

bartlantz commented
Status: Active » Needs review
StatusFileSize
new clean_up_api_docs_profiles_directory-1355362-2.patch3.06 KB

Here's a patch to clean up API docs in the profiles directory.
Thanks,
Bart

bartlantz’s picture

bartlantz commented
StatusFileSize
new clean_up_api_docs_profiles_directory-1355362-3.patch2.87 KB

Here's a better patch, in the previous one I didn't document the hook definitions correctly. This patch replaces the one above and adds this line to the hook definitions:
Implements hook_form_FORM_ID_alter() for install_configure_form()

Thanks,
Bart

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Needs review » Needs work 
+++ b/profiles/testing/testing.profile
@@ -1,2 +1,6 @@
+ * Tests for installation profiles.

That's not correct. It's more the other way around.
"Installation profile for tests."

-11 days to next Drupal core point release.

bartlantz’s picture

bartlantz commented
Status: Needs work » Needs review
StatusFileSize
new clean_up_api_docs_profiles_directory-1355362-4.patch2.87 KB

Aha, that makes more sense! Okay, I fixed the description of the testing.profile. It now says, "Installation profile for tests."

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Needs review » Reviewed & tested by the community

Cool stuff!
Thanks for the quick re-roll.

aspilicious’s picture

aspilicious commented

I can't open the last patch o_O

jhodgdon’s picture

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

uh oh, me neither. bartlantz I hope you still have a copy of it...

bartlantz’s picture

bartlantz commented
StatusFileSize
new clean_up_api_docs_profiles_directory-1355362-8.patch2.87 KB

Yes, I still do, here it is.

bartlantz’s picture

bartlantz commented
Status: Needs work » Needs review
StatusFileSize
new clean_up_api_docs_profiles_directory-1355362-9.patch2.87 KB

whoops, forgot to change status to needs review. Was the file unavailable on drupal or is there a problem with the patch?

aspilicious’s picture

aspilicious commented

You don't need to upload it again when you forgot to change the status, it will test the last patch after changing the status.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

barlantz: it looks like Drupal.org lost the previous upload. Not sure why. Thanks for re-uploading.

jhodgdon’s picture

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

This looks fine, thanks!

webchick’s picture

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

I found a couple of minor things, which I could just fix on commit, but since this is your first(?) core patch, I wanted to give you the opportunity to do so. :)

+++ b/profiles/minimal/minimal.installundefined
@@ -1,9 +1,16 @@
+/**
+ * @file
+ * Install, update and uninstall functions for the the minimal install profile.
+ */
+

This actually adds one more newline than we need.

+++ b/profiles/minimal/minimal.profileundefined
@@ -1,7 +1,12 @@
+ * Enables modules and site configuration for a minimal site installation.
+ **/

one too many *s on the */ line.

+++ b/profiles/testing/testing.profileundefined
@@ -1,2 +1,6 @@
 
+/**
+ * @file

Here you've left a newline between <?php and /** but elsewhere you don't. It should be consistent.

bartlantz’s picture

bartlantz commented
Status: Needs work » Needs review
StatusFileSize
new clean_up_api_docs_profiles_directory-1355362-14.patch2.86 KB

Cool, thanks! Here's a patch with the fixes.

jhodgdon’s picture

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

Looks good, thanks! The three issues identified in #13 (which I missed, sorry!) are fixed.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Assigned: bartlantz » jhodgdon

Moving to Jennifer's RTBC pile. :)

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Version: 8.x-dev » 7.x-dev
Assigned: jhodgdon » Unassigned
Status: Reviewed & tested by the community » Patch (to be ported)
Issue tags: +Needs backport to D7

Committed to 8.x! Moving to 7.x for backport.

xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: +Novice

Backporting this would be a good novice issue. Should be simple; I didn't see any D8-specific changes on a quick skim.

bartlantz’s picture

bartlantz commented
Assigned: Unassigned » bartlantz
Status: Patch (to be ported) » Needs review
StatusFileSize
new clean_up_api_docs_profiles-1355362-19.patch2.83 KB

Here's the patch backported to 7.x.

thanks,
Bart

xjm’s picture

xjm
she/her
Primary language English
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Novice

Backport looks fine too. Thanks @bartlantz!

jhodgdon’s picture

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

Committed to 7.x -- Another API docs cleanup done! :)

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