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

(REF) dev/core#1744 - Cleanup event naming #17240

Merged
merged 4 commits into from
May 6, 2020

Conversation

totten
Copy link
Member

@totten totten commented May 6, 2020

Overview

This is a cleanup in that it consolidates the code-style by which certain events are organized.

Discussion/rationale: https://lab.civicrm.org/dev/core/-/issues/1744

Before

For any given event, there are up to 3 distinct names -- the class-name (eg TokenRegisterEvent), the event-name (eg civi.token.list), and the constant-name (eg TOKEN_REGISTER).

The usage of event-name vs constant-name is inconsistent. You find numerous examples of both these formulations in civicrm-core

// Reference an event directly by its literal name
Civi::dispatcher()->addListener('civi.token.list', $callback);
Civi::dispatcher()->dispatch('civi.token.list', $event);

// Reference an event indirectly by a constant
Civi::dispatcher()->addListener(Civi\Token\Events::TOKEN_REGISTER, $callback);
Civi::dispatcher()->dispatch(Civi\Token\Events::TOKEN_REGISTER, $event);

After

The constants are mostly removed/deprecated. This string-literal formulation is canonical:

Civi::dispatcher()->addListener('civi.token.list', $callback);
Civi::dispatcher()->dispatch('civi.token.list', $event);

Technical Details

Normalizing/consolidating this involved a few changes:

  • (REF) Search/replace - Simply substitute the constant with its value
  • (NFC) Mark the old constant as @deprecated, but retain it for any third-party code or any unrecognized edge-cases.
  • (NFC) Consolidate docblocks - Each event had a constant (TOKEN_REGISTER) and a class (TokenRegisterEvent). Docblocks describing the event could be on either. Any docblocks on the constant have been moved into the corresponding class.

@civibot
Copy link

civibot bot commented May 6, 2020

(Standard links)

@civibot civibot bot added the master label May 6, 2020
@mattwire
Copy link
Contributor

mattwire commented May 6, 2020

test fail unrelated

@mattwire mattwire merged commit ccc8a89 into civicrm:master May 6, 2020
totten added a commit to totten/civicrm-dev-docs that referenced this pull request May 7, 2020
@totten totten deleted the master-event-names branch May 7, 2020 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants