Skip to:
Content

BuddyPress.org

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#6051 closed enhancement (fixed)

Rename bp-core/admin/ files to avoid confusion with non-admin files sharing the same name

Reported by: DJPaul Owned by: DJPaul
Milestone: 2.3 Priority: normal
Severity: normal Version:
Component: Administration Keywords: 2nd-opinion
Cc:

Description

When I try to open a file by name in my IDE, I type in e.g. "bp-core-actions" and I get two choices: the one in the root of the bp-core folder, and the one inside bp-core/admin/.

Can we rename the files inside the admin folder to "bp-admin-NAME.php"? For example:

`
bp-core/admin/bp-core-actions.php -> bp-core/admin/bp-admin-actions.php
bp-core/admin/bp-core-functions.php -> bp-core/admin/bp-admin-functions.php
`

"bp-core-admin-functions.php" looks too long to me, but I could live with that if that's what we decided.

All other components' admin code is in a "-admin.php" file within that component, and that's fine, as I think the only component with multiple files for wp-admin stuff is core (partly because it registers all the actions/loads the code that the other components rely on).

A quick rename would de-duplicate the file names and make clearer the context they're loaded in.

Change History (8)

#1 @DJPaul
5 years ago

  • Milestone changed from Awaiting Review to 2.3
  • Owner set to DJPaul
  • Status changed from new to assigned

#2 @DJPaul
5 years ago

  • Keywords 2nd-opinion added

I want to bump this and double-check how people feel about it before I go ahead and rename these admin files.

#3 @r-a-y
5 years ago

The proposed renaming convention looks good to me!

#4 @boonebgorges
5 years ago

+1. Just make sure to svn mv so we keep revision history.

#5 @DJPaul
5 years ago

I looked at this and decided we need to go with bp-core-admin-functions.php (for example), because otherwise we'd up with new identically-named conflicts between the Groups and Core components.

#6 @djpaul
5 years ago

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

In 9503:

Rename component admin files to avoid confusion with non-admin files sharing the same name.

This rename de-duplicates the file names and make the context they're loaded in clearer.
This is a change for developer convenience. e.g. some IDEs will offer a shortcut to opening the files by name, but is very confusing when two identically-named files are listed in the results with, usually, no quick way of disambiguating them.

Fixes #6051

#7 @johnjamesjacoby
5 years ago

If any IDE cannot distinguish the difference between filenames in separate directories, that's a tool problem, and one that should not dictate how we name our files.

In my experience, the IDE I use has no problem with opening files in different directories, and I expect a lack of support for a global rename to shorten filenames (likely resulting in more overlap between components) even though one benefit would be more filenames visible in the tabulated area of my IDE where there is also a UI breadcrumb to the open file to dispel any confusion.

At the risk of stating the very obvious, I am 100% in favor of using tools to improve the readability and usability BuddyPress, and I don't disagree that this changeset fixes this issue which it sounds annoys more than 1 of us.

Given the recent class break-ups in #6083 and discussion around renaming files in Slack last week, this seems like a hot enough topic to try to define what our goals are with filenames and take steps to achieve them in 2.3.

#8 @johnjamesjacoby
5 years ago

Forgot to mention, this rename made patching #6244 confusing. Imath's patch for trunk didn't apply cleanly to 2.2, and I wasn't aware the filenames had changed so recently. That's my fault for not seeing the changeset, but I was pretty confused for a minute there.

Note: See TracTickets for help on using tickets.