Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a hook documentation #1861

Merged
merged 1 commit into from
Sep 12, 2024
Merged

Add a hook documentation #1861

merged 1 commit into from
Sep 12, 2024

Conversation

tleon
Copy link
Contributor

@tleon tleon commented Sep 9, 2024

Questions Answers
Branch? 9.x
Description? Documentation on how to add a new hook

@tleon tleon self-assigned this Sep 9, 2024
@github-actions github-actions bot added the 8.x label Sep 9, 2024
contribute/contribute-pull-requests/add_hook.md Outdated Show resolved Hide resolved
contribute/contribute-pull-requests/add_hook.md Outdated Show resolved Hide resolved

## 1) Add a call to Hook::exec()

As you can see on this [PR](https://github.com/PrestaShop/PrestaShop/pull/34431/files) Adding a simple Hook::exex() statement is enought to add a new hook. In order for it to be complete, there is a couple more steps to do.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
As you can see on this [PR](https://github.com/PrestaShop/PrestaShop/pull/34431/files) Adding a simple Hook::exex() statement is enought to add a new hook. In order for it to be complete, there is a couple more steps to do.
As you can see on this [PR](https://github.com/PrestaShop/PrestaShop/pull/34431/files), adding a simple Hook::exex() statement is enough to add a new hook. In order for it to be complete, there is a couple more steps to do.

Copy link
Contributor

@Hlavtox Hlavtox Sep 10, 2024

Choose a reason for hiding this comment

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

exec

Copy link
Contributor

Choose a reason for hiding this comment

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

I would be more precise here. In the shown example, you don't write about "adding a hook" but adding code that executes the hook in this place. That doesn't mean the hook is fully added to the system if you add this code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the only way I was told how to do it maybe you can enlight me ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think @kpodemski means that adding the Hook::exec() (part 1) will trigger the hook, but if you dont do the parts 2 and parts 3 it will work only partially

contribute/contribute-pull-requests/add_hook.md Outdated Show resolved Hide resolved
Copy link
Contributor

@kpodemski kpodemski left a comment

Choose a reason for hiding this comment

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

Good idea to have it as a separate article. I added my feedback.

@@ -0,0 +1,14 @@
# How To add a new Hook
Adding a new hook is quite a simple process. It can be done in only tree steps.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Adding a new hook is quite a simple process. It can be done in only tree steps.
Adding a new hook is a process that can be done in only three steps.

We should refrain from using "simple", "easy", etc., in the documentation. It might not be that simple for some :)


## 1) Add a call to Hook::exec()

As you can see on this [PR](https://github.com/PrestaShop/PrestaShop/pull/34431/files) Adding a simple Hook::exex() statement is enought to add a new hook. In order for it to be complete, there is a couple more steps to do.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would be more precise here. In the shown example, you don't write about "adding a hook" but adding code that executes the hook in this place. That doesn't mean the hook is fully added to the system if you add this code.

contribute/contribute-pull-requests/add_hook.md Outdated Show resolved Hide resolved
@tleon tleon requested review from a team and removed request for a team September 11, 2024 09:10
@Hlavtox
Copy link
Contributor

Hlavtox commented Sep 11, 2024

It would be good to add "what happens if X". The hook doesn't have to be in the hooks table to work, it will be added automatically when first module registers on it. However, it can be done only programatically, not by using the backoffice.

contribute/contribute-pull-requests/add_hook.md Outdated Show resolved Hide resolved
contribute/contribute-pull-requests/add_hook.md Outdated Show resolved Hide resolved
contribute/contribute-pull-requests/add_hook.md Outdated Show resolved Hide resolved
contribute/contribute-pull-requests/add_hook.md Outdated Show resolved Hide resolved
contribute/contribute-pull-requests/add_hook.md Outdated Show resolved Hide resolved
@kpodemski
Copy link
Contributor

I added final suggestions @tleon @Hlavtox

Copy link
Contributor

@matks matks left a comment

Choose a reason for hiding this comment

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

Good for me

Waiting for @kpodemski last check ;)

@kpodemski kpodemski merged commit 3ae473c into PrestaShop:8.x Sep 12, 2024
2 checks passed
@kpodemski
Copy link
Contributor

thanks @tleon :-)

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

Successfully merging this pull request may close these issues.

5 participants