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

Fixed hook name #280

Merged
merged 1 commit into from Aug 9, 2019

Conversation

@mickaelandrieu
Copy link
Member

commented May 29, 2019

No description provided.

@matks

This comment has been minimized.

Copy link
Contributor

commented May 31, 2019

Should'nt we fix it first in hook.xml and SQL upgrade file ?

@mickaelandrieu

This comment has been minimized.

Copy link
Member Author

commented May 31, 2019

I can't because docs and core are not in the same repositories ;)

Am I repeating myself? 👼

@matks

This comment has been minimized.

Copy link
Contributor

commented May 31, 2019

I can't because docs and core are not in the same repositories ;)

Am I repeating myself? 👼

I dont see how this prevents us from doing a PR in the Core repository fixing the hook name in both the hook.xml file and the dedicated SQL upgrade first.

I can show you if you want :trollface:

@mickaelandrieu

This comment has been minimized.

Copy link
Member Author

commented May 31, 2019

I dont see how this prevents us from doing a PR in the Core repository fixing the hook name in both the hook.xml file and the dedicated SQL upgrade first.

Think about it: if the docs were inside the core repository, I could address it in one step. Even if we do a PR right now, it will be merged in 1.7.7, so in almost 1 year for people :p

I can show you if you want :trollface:

Feel free to do it, I'm focused on docs now 🤣

@matks

This comment has been minimized.

Copy link
Contributor

commented Jun 3, 2019

Even if we do a PR right now, it will be merged in 1.7.7, so in almost 1 year for people :p

The fix needs only to target the hook.xml file (not critical) and the SQL upgrade (critical, but no code)
Considering this avoids some confusion about the hooks, and that could justify a fix in 1.7.6.x

ping @eternoendless

@mickaelandrieu

This comment has been minimized.

Copy link
Member Author

commented Jun 12, 2019

LGTM for the docs part ?

@eternoendless

This comment has been minimized.

Copy link
Member

commented Aug 9, 2019

Thank you @mickaelandrieu

@eternoendless eternoendless merged commit ab5dd9a into master Aug 9, 2019
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@eternoendless eternoendless deleted the mickaelandrieu-patch-2 branch Aug 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.