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 phase_order rake task and update README #123

Open
wants to merge 1 commit into
base: mobby/james_phases
Choose a base branch
from

Conversation

kkohrt
Copy link

@kkohrt kkohrt commented Jan 9, 2025

The most confusing part about the proposed dependency graph configuration was understanding what the default order would be. And the biggest risks seemed to be in accidentally creating a circular dependency or specifying a dependency incorrectly. This change addresses some of those issues as follows"

  1. Updates the phases class to coerce phase names into symbols, and dependencies into arrays of symbols
  2. Introduces a rake task that prints out the order of phases and their dependencies.

It also does a major overhaul to the README that reflects the additional information and insights I gained while working in the code and testing the dependency features that I wish would have been in the README to begin with. Also: Updates the example rake tasks to a more modern Rails format, and the Procore logo to use the procore-oss source.

Output of new rake task (when run from /spec/dummy test application):

$ bundle exec rake handcuffs:phase_order

Configured Handcuffs phases, in order, are:
  1. pre_restart, requires: (nothing)
  2. post_restart, requires: pre_restart
  3. data_migrations, requires: pre_restart

I get the same output when I "mangle" the configuration as follows

  config.phases = {
    # Prevent running post_restart migrations if there are outstanding
    # pre_restart migrations
    post_restart: :pre_restart,
    # Require pre_restarts before data_migrations, but do not enforce ordering
    # between data_migrations and post_restarts
    data_migrations: 'pre_restart',
    # pre_restarts have no prerequisite phases
    'pre_restart' => []
  }

Checklist:

  • I have updated the necessary documentation
  • I have signed off all my commits as required by DCO
  • My build is green

@kkohrt kkohrt requested review from a team as code owners January 9, 2025 04:55
@kkohrt kkohrt force-pushed the kgk/phase_order_rake_and_readme branch 7 times, most recently from 733b010 to 959876a Compare January 9, 2025 14:37
Signed-off-by: Kevin Kohrt <kevin.kohrt@procore.com>
@kkohrt kkohrt force-pushed the kgk/phase_order_rake_and_readme branch from 959876a to febb9dd Compare January 9, 2025 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant