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.

Sign up for GitHub

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

Jump to bottom

Allow hook priority #244

Open
claudiu-cristea opened this issue Nov 22, 2021 · 1 comment
Open

Allow hook priority #244

claudiu-cristea opened this issue Nov 22, 2021 · 1 comment

Comments

@claudiu-cristea
Copy link

claudiu-cristea commented Nov 22, 2021 •
edited

Problem

I would like to be able to determine the order in which some hooks are running (see also #113). As hooks are relying on Symfony event dispatcher, which supports priorities, this should be doable.

Proposal

Allow to pass an additional and optional priority integer value to the @hook annotation, for instance

@hook post-command commandname 300
@greg-1-anderson
Copy link
Member

Only some of the hooks rely on the event dispatcher. I was initially reluctant to add the complexity of priority, opting instead for pre and post phases. Would it make sense to use the event dispatcher all the time, and re-implement pre and post as shorthands for priority values? I don't know. I remain ambivalent.

Why do you need hook priority?

Jelle-S added a commit to district09/robo-digipolis-helpers that referenced this issue Nov 7, 2022
@Jelle-S
Refactor the whole package to use events
a1bf157 
Because of consolidation/annotated-command#273,
which technically speaking contains a BC break, it was made clear for us
that we're not using Robo or the consolidation packages as they were
intended. In an effort to keep this package as extendible as possible,
and keeping the same philosophy as before, we switched to an event-based
system. Basically every step in a command fires an event. The event
listener returns a `HandlerWithPriority`, which it turn returns a
`TaskInterface` to be executed for this step in the command. We made our
own implementation of event priorities, since the default implementation
used in Robo doesn't support them
(consolidation/annotated-command#244).

I tried to document everything as well as possible in the README, but it
might (probably will) need some lovin'.
Jelle-S added a commit to district09/robo-digipolis-helpers that referenced this issue Nov 7, 2022
@Jelle-S
Refactor the whole package to use events
6b756a6 
Because of consolidation/annotated-command#273,
which technically speaking contains a BC break, it was made clear for us
that we're not using Robo or the consolidation packages as they were
intended. In an effort to keep this package as extendible as possible,
and keeping the same philosophy as before, we switched to an event-based
system. Basically every step in a command fires an event. The event
listener returns a `HandlerWithPriority`, which it turn returns a
`TaskInterface` to be executed for this step in the command. We made our
own implementation of event priorities, since the default implementation
used in Robo doesn't support them
(consolidation/annotated-command#244).

I tried to document everything as well as possible in the README, but it
might (probably will) need some lovin'.

This package contains default event handlers with some sensible (to us)
defaults. You can prevent these handlers from being executed by writig
your own handler with a higher priority (lower number) which does its
thing and then calls `$event->stopPropagation()`.
Jelle-S added a commit to district09/robo-digipolis-helpers that referenced this issue Nov 15, 2022
@Jelle-S
Refactor the whole package to use events
4a2b169 
Because of consolidation/annotated-command#273,
which technically speaking contains a BC break, it was made clear for us
that we're not using Robo or the consolidation packages as they were
intended. In an effort to keep this package as extendible as possible,
and keeping the same philosophy as before, we switched to an event-based
system. Basically every step in a command fires an event. The event
listener returns a `HandlerWithPriority`, which it turn returns a
`TaskInterface` to be executed for this step in the command. We made our
own implementation of event priorities, since the default implementation
used in Robo doesn't support them
(consolidation/annotated-command#244).

I tried to document everything as well as possible in the README, but it
might (probably will) need some lovin'.

This package contains default event handlers with some sensible (to us)
defaults. You can prevent these handlers from being executed by writig
your own handler with a higher priority (lower number) which does its
thing and then calls `$event->stopPropagation()`.
@Jelle-S Jelle-S mentioned this issue Nov 15, 2022
Merged
Jelle-S added a commit to district09/robo-digipolis-helpers that referenced this issue Nov 24, 2022
@Jelle-S
Merge branch 'develop'
2f056a2 
Merge details:

commit 8c4bbc2
Author: Jelle Sebreghts <sebreghts.jelle@gmail.com>
Date:   Thu Nov 17 09:38:26 2022

    Fix call to undefined method

    Call to undefined method
    DigipolisGent\Robo\Drupal8\Robo\Plugin\Commands\DigipolisDrupal8DeployCommand::taskSsh()

commit d8c7cbb
Author: Jelle Sebreghts <sebreghts.jelle@gmail.com>
Date:   Thu Nov 17 09:34:21 2022

    Improve documentation and default behavior

    Improve documentation by referencing the robo-digipolis-deploy
    documentation about providing database connection details.

    Fix a broken anchor link.

    By default, create a backup and rollback during deploy. Add
    documentation for the option in the `properties.yml` documentation.

commit 4a5d856
Author: Jelle Sebreghts <sebreghts.jelle@gmail.com>
Date:   Tue Nov 15 15:51:51 2022

    Fix typehint in docblock

commit 4a2b169
Author: Jelle Sebreghts <sebreghts.jelle@gmail.com>
Date:   Mon Nov 7 17:06:43 2022

    Refactor the whole package to use events

    Because of consolidation/annotated-command#273,
    which technically speaking contains a BC break, it was made clear for us
    that we're not using Robo or the consolidation packages as they were
    intended. In an effort to keep this package as extendible as possible,
    and keeping the same philosophy as before, we switched to an event-based
    system. Basically every step in a command fires an event. The event
    listener returns a `HandlerWithPriority`, which it turn returns a
    `TaskInterface` to be executed for this step in the command. We made our
    own implementation of event priorities, since the default implementation
    used in Robo doesn't support them
    (consolidation/annotated-command#244).

    I tried to document everything as well as possible in the README, but it
    might (probably will) need some lovin'.

    This package contains default event handlers with some sensible (to us)
    defaults. You can prevent these handlers from being executed by writig
    your own handler with a higher priority (lower number) which does its
    thing and then calls `$event->stopPropagation()`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@claudiu-cristea @greg-1-anderson