Skip to content
This repository has been archived by the owner on Aug 3, 2023. It is now read-only.

Given the plugin a shape #20

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

szepeviktor
Copy link

@szepeviktor szepeviktor commented Feb 15, 2021

  • PSR-4
  • PSR-12
  • HookAnnotation

Closes #18

Please like, share and subscribe!

@szepeviktor szepeviktor changed the title Given the plugins a shape Given the plugin a shape Feb 15, 2021
@curquiza
Copy link
Member

Hello again @szepeviktor
Remove the Draft PR status when you will need a review 🚀 We will try to be as reactive as possible, we are sorry if it takes time sometimes, it's often the rush for our team! 😄

@szepeviktor
Copy link
Author

@timothyjensen Look! 👀 It is like a rebirth.

@szepeviktor
Copy link
Author

Development continues 🚀 e.g. with a search for container ...

@szepeviktor
Copy link
Author

@curquiza I was struggling with the injector for days. Now it is in place.

Could you read my code?

@szepeviktor
Copy link
Author

Please be aware that I've already received feedback like

Viktor, this is a WordPress site not a Symfony application

:)

@curquiza
Copy link
Member

curquiza commented Mar 9, 2021

Hello @szepeviktor I'll let @eskombro check this PR since he was one of the authors of this repo.
He has a lot to do before so it might take time 🙂

@curquiza curquiza requested a review from eskombro March 9, 2021 21:47
@szepeviktor
Copy link
Author

I just want a confirmation that this code meets your expectations.

Copy link
Member

@eskombro eskombro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @szepeviktor!

I am really excited about this huge improvement you are introducing to this plugin and repo, thanks a lot! The project structure and the code readability and in-code documentation are just great!

I checked what you are doing on this PR, but sadly I couldn't test it because it is not possible yet to activate the plugin on the wp-admin dashboard, I got a fatal error (or am I missing something?). In case you think I should be able to run it already, would you mind giving me an idea of the steps you follow to install / activate?

I also got curious about your extension phpstan-wordpress. I see it has got some attention on GitHub, congrats! (I just gave it one more star ⭐ )

My advice would be to try and finish a very basic plugin, something like an MVP that we can run and test, and after that introduce the changes gradually, instead of ending up with one single big change to switch it all at once, what do you think?

Anyways, amazing work, thanks again! 🎉

@szepeviktor
Copy link
Author

szepeviktor commented Mar 11, 2021

All right - all right.
I'm not a developer, I support development with technologies.

Running the plugin is the last step of me writing code.
I was just afraid that OOP technologies won't be accepted in a WordPress plugin.

@eskombro
Copy link
Member

Running the plugin is the last step of me writing code.

Oh, that's fine, I just wanted to be sure that I wasn't missing something :)

I was just afraid that OOP technologies won't be accepted in a WordPress plugin.

I don't see any reason why it wouldn't work, and I have seen a few examples (even some small articles talking about it like this one)but this is why I think it would be better to try and run a very basic version of it and make sure everything works together, and then add all the specific functionality, so in case there is a problem with the main plugin structure or some other issue we are not thinking about, we can realize that and correct it early enough!

@szepeviktor
Copy link
Author

I am very 😢 sad, Samuel.

That article.

function __construct( $config ) {
	$this->api_key = $config['api_key'];

	add_filter( 'the_content', array( $this, 'form' ) );
	add_action( 'wp_enqueue_scripts', array( $this, 'assets' ) );
	add_action( 'wp_ajax_msf_form_submit', array( $this, 'submissionHandler' ) );
	add_action( 'wp_ajax_nopriv_msf_form_submit', array( $this, 'submissionHandler' ) );
}

This is also procedural code masqueraded in a class.

Very sad. People are unable to think.

@szepeviktor
Copy link
Author

szepeviktor commented Mar 11, 2021

so in case there is a problem with the main plugin structure or some other issue we are not thinking about

Before writing code I've made sure I rule the plugin. All its functionality is in my hand, in a tight grip.
That is prerequisite of sustainable software.

@eskombro
Copy link
Member

eskombro commented Mar 11, 2021

I am very 😢 sad, Samuel.
That article.

This is also procedural code masqueraded in a class.

Don't be sad :D

This article is indeed just a guide for people who want to transform their procedural way of coding plugins into OOP, so it is focused on that transition. But it just a simple example.

I don't see any reason to be worried about any OOP principles causing problems on Wordpress plugin development, I have seen polymorphism, inheritance, interfaces, association, abstraction... So it feels safe to me, do you have any specific concern?

Also, don't be sad :)

@szepeviktor
Copy link
Author

do you have any specific concern?

Yes. Reader are made believe that putting functions into a class is called OOP.

An object is a sovereign entity able to do all of its duties without any outside help.

@eskombro
Copy link
Member

eskombro commented Mar 11, 2021

do you have any specific concern?

Yes. Reader are made believe that putting functions into a class is called OOP.

An object is a sovereign entity able to do all of its duties without any outside help.

Hahaha, I agree, but let me rephrase:

Do you have any concern regarding the MeiliSearch plugin implementation concerning OOP? :)

@szepeviktor
Copy link
Author

szepeviktor commented Mar 11, 2021

See the immutable configuration class, that is OOP.

@szepeviktor
Copy link
Author

Do you have any concern regarding the MeiliSearch plugin implementation concerning OOP? :)

No, I strive to eliminate all traces of "WordPress programing" as much as I can.
The hook (filter/action) system is inherently procedural so we will have fake classes with hooked methods.

@curquiza
Copy link
Member

curquiza commented Apr 6, 2021

Hello @szepeviktor!
Do you need help to finalize your PR? You can pass it as Ready for review when you think this is over 🙂

@szepeviktor
Copy link
Author

Yes, I need to wake up!!

@curquiza
Copy link
Member

curquiza commented Apr 6, 2021

Take your time, it's ok 🙂 I just needed to know!

@szepeviktor
Copy link
Author

I have no schedule ❌📆
Everything is ASAP.

@szepeviktor
Copy link
Author

szepeviktor commented Apr 7, 2021

Added missing bits and pieces.
Tomorrow I will run the code for the 1st time 🥇

kép

@thijndehaas
Copy link

Should the post not be deleted when it is changes from published to draft?

if ($post->post_status !== 'publish') {
    return $this->deletePostFromIndex($post->ID);
}

$this->indexPost($post);

@szepeviktor
Copy link
Author

szepeviktor commented Mar 28, 2022

Should the post not be deleted when it is changes from published to draft?

You are right.
I've sent this PR without knowing what is the goal of this plugin.
When I work I always make plans (docs) before writing code.

@curquiza
Copy link
Member

Hello @szepeviktor and @thijndehaas

FYI we chose to stop working on the meilisearch-wordpress plugin. It does not mean we will not continue the work in the future, but this is currently not the priority :)

Sorry for this, and thanks for your work and comments :)

@szepeviktor
Copy link
Author

Thank you for that Clémentine!
It opens up the possibility for me to publish a plugin.

@curquiza
Copy link
Member

curquiza commented Mar 29, 2022

Feel completely free to publish the plugin on your side! 😄 We appreciate our community's work around Meilisearch!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WP.org compatible name
4 participants