Skip to:
Content

BuddyPress.org

Opened 8 years ago

Closed 8 years ago

#7239 closed enhancement (fixed)

Wrong naming on Friend component

Reported by: espellcaste's profile espellcaste Owned by: tw2113's profile tw2113
Milestone: 2.7 Priority: normal
Severity: normal Version:
Component: Friends Keywords:
Cc:

Description

I noticed this error when studying BuddyPress code.

https://github.com/buddypress/BuddyPress/blob/master/src/bp-friends/bp-friends-loader.php#L20

This is a good opportunity for me to finally learn to send patches. So a patch is coming along. =)

Attachments (6)

7239.diff (8.4 KB) - added by espellcaste 8 years ago.
7239-1.patch (17.2 KB) - added by espellcaste 8 years ago.
7239-2.patch (24.4 KB) - added by espellcaste 8 years ago.
7239-3.patch (25.7 KB) - added by espellcaste 8 years ago.
7239-4.patch (18.1 KB) - added by espellcaste 8 years ago.
7239-5.patch (3.5 KB) - added by espellcaste 8 years ago.

Download all attachments as: .zip

Change History (23)

#1 @mercime
8 years ago

Go for it! :)

@espellcaste
8 years ago

#2 @espellcaste
8 years ago

This patch is wrong, sorry!

@espellcaste
8 years ago

#3 @espellcaste
8 years ago

Not yet! :/ :(

@espellcaste
8 years ago

#4 @espellcaste
8 years ago

Not sure why it added the class twice... :( Trying to fix it, I can do it!

@espellcaste
8 years ago

#5 @espellcaste
8 years ago

I don't know what it is, but the patches I create keeps coming with duplicate files, for example, there is two files with the same class in 7239-3.patch.

#6 @DJPaul
8 years ago

  • Keywords reporter-feedback added

@espellcaste We're happy to help you go over patch creation. There's minimal docs over on the WordPress site, which has the basics (it'll be different if you're using an app): https://make.wordpress.org/core/handbook/tutorials/trac/submitting-a-patch/

If your timezone overlaps, reach out to someone in our Slack room, or I can email you and try to help.

(I suspect the file appears twice because you've deleted the file and re-added it, rather than use svn mv, but we can figure that out).

My question for you is: what's the purpose of this ticket? It only says "Wrong naming on Friend component", not what is wrongly named. In the patch, I see some PHPDoc changes, as well as renaming a file. Can you add some more information please? Thanks!

@espellcaste
8 years ago

This ticket was mentioned in Slack in #buddypress by espellcaste. View the logs.


8 years ago

#8 @espellcaste
8 years ago

I think now I got it right! Patch 7239-4 is the correct one. Please, disregard the others! Those were tests!

@DJPaul @hnla helped me on Slack on the problem.

Regarding the purpose of this ticket:

1st - For me to finally learn to send patches. I'm not a big fan of svn, neither have I a lot of knowledge of it. But every now and then I postpone contributing because I need to set a time to learn it.

2nd - I guess the ticket name was poorly formed, but basically I want to correct this error:

https://github.com/buddypress/BuddyPress/blob/master/src/bp-friends/bp-friends-loader.php#L20

The PHPDoc name "bp-forums" is in the wrong place, as it is in the friends component.

Also there are some errors that you can see in the patch.

Also, I'm standardizing the beginning of the PHPDoc to: "Set up the bp-nameofcomponent component."

And lastly, I'm adding support for autoload to bp_forums_component loader.

Last edited 8 years ago by espellcaste (previous) (diff)

#9 @boonebgorges
8 years ago

Thanks for the patch, @espellcaste!

For future reference, you never have to touch svn to submit a patch here. You can use Git. Here's a very short description of the process:

  1. Create a clone of the official Git repo: git clone git://buddypress.git.wordpress.org. The master branch is equivalent to SVN trunk
  2. Do your fixes
  3. To generate a patch, use git diff --no-prefix > /path/to/file.patch

See also https://make.wordpress.org/core/handbook/contribute/git/.

#10 @tw2113
8 years ago

  • Owner set to tw2113
  • Status changed from new to assigned

#11 @espellcaste
8 years ago

Thanks @boonebgorges. I did some tests here and it works! :D

#12 @espellcaste
8 years ago

  • Keywords reporter-feedback removed

#13 @tw2113
8 years ago

I like all parts of the the patch EXCEPT the autoloading portion. I think getting that component piece loaded that way warrants its own ticket, and this ticket can be focused on just the docs updates, which appears to be the primary original focus anyway.

@espellcaste can you provide a diff with just the phpdoc portions and I'll get that committed with props for you. If @DJPaul and/or @boonebgorges are fine with the autoloading part accompanying the patch, then i'll just go with revision 4

#14 @boonebgorges
8 years ago

@espellcaste Thanks for the patch!

I agree with @tw2113 that the bp-forums autoload stuff should not be included here. For one thing, it's broken :) - the autoloader in bp-loader.php doesn't expect bp-forums/classes to be autoloaded. But also, it's a separate issue, and should be handled separately from the documentation bits.

@tw2113 Please go ahead with the rest of this patch as you see fit. Thanks to both of you!

#15 @tw2113
8 years ago

I'll extract the appropriate parts and get those parts committed.

#16 @espellcaste
8 years ago

@boonebgorges Got it! ;)

@tw2113 Thanks!

@espellcaste
8 years ago

#17 @tw2113
8 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 11064:

Fixs a number of inconsistencies in PHPDoc comments for various component loaders. Props @espellcaste.

Fixes #7239.

Note: See TracTickets for help on using tickets.