Skip to:
Content

BuddyPress.org

Opened 2 years ago

Closed 17 months ago

#7604 closed enhancement (fixed)

Merge WP-CLI commands into core

Reported by: boonebgorges Owned by: djpaul
Milestone: 3.0 Priority: normal
Severity: normal Version:
Component: Core Keywords:
Cc: stephen@…, contato@…

Description

@espellcaste has recently taken over primary development on the wp-cli-buddypress repo https://github.com/buddypress/wp-cli-buddypress

When the commands reach a certain level of coverage (like, maybe, CRUD methods for all content types), we should consider merging into the main BuddyPress package. Let's use this ticket to hash out the following:

(a) whether this is a good idea
(b) what criteria we should use to decide when to merge
(c) how the merge should work (how to isolate CLI functionality, whether to merge into the main svn repo or to pull from GitHub at build time, etc)

Change History (25)

#2 @netweb
2 years ago

  • Cc stephen@… added

#3 @espellcaste
2 years ago

  • Cc contato@… added
  • Type changed from defect (bug) to enhancement

Thank you for considering this. This is what I think:

(a) Yes! :)
(b) Basic CRUD as you mentioned with proper behat tests.
(c) Create a cli folder inside BuddyPress. Add it into the main svn repo. This will be part of BuddyPress like any other component, be that cli or a rest API.

By the way, after merging this into BP core, add here on Trac, a component called WP CLI for future commands or possible bugs. This will allow better organization. Thank you! :)

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


22 months ago

#5 follow-up: @DJPaul
22 months ago

You think it's best to keep all these commands in a new folder, and not put each WP-CLI command file into the existing component subfolders/naming convention?

#6 in reply to: ↑ 5 @boonebgorges
22 months ago

Replying to DJPaul:

You think it's best to keep all these commands in a new folder, and not put each WP-CLI command file into the existing component subfolders/naming convention?

I don't know yet. I *don't* think it makes sense to think of CLI as another "component" - components are already pretty ill-defined, and CLI commands don't really even match that poor definition. So I think there are two options:

  1. introduce some sort of new organizational principle for stuff like this - maybe something PSR-4 compliant, or whatever
  2. put the individual CLI subcommand files in with their components

My only misgiving about 2 is that there are likely to be some parts of the CLI suite that shared between components, or are not really linked to a component. I suppose those could go in bp-core.

All told, I don't have a strong opinion. Whatever makes life easiest for devs.

Worth noting that we'll probably want to make a similar decision when/if we want to integrate some sort of API endpoints, so it may pay to think ahead somewhat.

#7 @espellcaste
22 months ago

If I'm correct, @r-a-y is working on a ticket that will load the component files only when activated. So this could be a problem if the cli files are loaded in their respective components.

Also, some cli commands need information that might not be only available component-wise, as @boonebgorges explained. Or might not be related to a component at all.

So I'd recommend adding everything together in a cli folder. Most likely how it is organized today but probably with better class/file naming.

#8 @r-a-y
22 months ago

The conditional component loading should only matter for frontend screen code. So unless you are utilizing a screen loader function (I don't think you are, but not sure), we should be fine.

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


21 months ago

#10 @DJPaul
21 months ago

Are you thinking we keep this project's home on Github, and every time we do a BuddyPress release, pull in the latest version? Or not?

#11 @boonebgorges
21 months ago

I don't have strong feelings about it. The biggest reason for keeping it on GitHub is to increase contributions from others, but practically speaking, I'm unsure whether that'll actually happen.

#12 @netweb
21 months ago

Code reviews are much easier to read on GitHub...

#13 @DJPaul
20 months ago

  • Milestone changed from Future Release to 3.0

#14 @DJPaul
20 months ago

@boonebgorges is and has been working with @espellcaste to review the code on Github. I will probably give it a quick review once everything is ready, but only lightly, because I'll assume Boone's done a good job.

Here is the milestone: https://github.com/buddypress/wp-cli-buddypress/milestone/2

@espellcaste Could you start planning how exactly the code will fit into the BP codebase?
Perhaps -- clone BuddyPress onto a temporary Github, create a branch, then commit the WP-CLI files -- then everyone can view the new file locations and offer any feedback.

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


20 months ago

#16 @espellcaste
20 months ago

As mentioned on Slack. I created a branch with wp-cli-buddypress to showcase how the integration might be possible.

https://github.com/renatonascalves/BuddyPress/tree/cli

#17 @DJPaul
20 months ago

Is this looking likely for 3.0, @espellcaste?

Can you update where you are with the wp-cli-buddypress project? What needs to be done before someone code-reviews for code inclusion?

#18 @espellcaste
19 months ago

@DJPaul There is nothing more to be done for now. The wp-cli-buddypress 1.6 version was finished and integrated into the test branch I created: https://github.com/buddypress/BuddyPress/compare/master...renatonascalves:cli

Right now, what's missing is a review of the team to make sure if the suggested integration place is good.

For example, take a look at how I decided to load the files, is that ok?
About the Behat tests, is that going to be included as well?

#19 @DJPaul
19 months ago

I'll try to have a look at the project this week.

I think we do want tests included since you wrote them (why not?!), let's put them in /tests/cli/. We can use BP's existing /composer.json to require the Behat projects, but we can worry about getting those running in Travis-CI later.

#20 @DJPaul
19 months ago

This is on track (har har!) for the next couple of weeks.

#21 @DJPaul
18 months ago

To update everyone, we're down to the final details on this. Just discussing SVN vs Git, maybe Composer, how to merge (or not) the code, etc. Should hopefully have consensus this week.

#22 @DJPaul
18 months ago

We're going to pull in BP-CLI as an external during our build process for 3.0.

There was indecision regarding best core implementation approach, and I suspect we'll bring it in fully in a release this year.

#23 @DJPaul
18 months ago

This is blocked by the BP WP CLI extension not containing any license information, and/or not having a recent tagged release to use.

#24 @DJPaul
17 months ago

Still waiting on the above. Need this by 19th April or we'll seriously have to consider not including this in BP 3.0, which would be a terrible shame.

#25 @djpaul
17 months ago

  • Owner set to djpaul
  • Resolution set to fixed
  • Status changed from new to closed

In 11978:

Add BuddyPress WP-CLI extension

For 3.0.0, the BP WP-CLI extension is added to the build package version of BuddyPress.
In a future release, its code will be migrated into BuddyPress core itself.

See https://github.com/buddypress/wp-cli-buddypress

Fixes #7604

Huge props to espellcaste, and boonebgorges :)

Note: See TracTickets for help on using tickets.