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

Allow specification of custom checklists from inside of client repo #20

Merged
merged 5 commits into from
Jan 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ cache:

before_install:
- gem update --system
- gem install bundler
- gem install bundler:1.16.3

script:
- bundle exec rubocop
Expand Down
38 changes: 38 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,44 @@ Use `GIT_DIR` to run a local copy of `pr-checklist` or `deploy-complexity` again
GIT_DIR=../repo bundle exec ./exe/pr-checklist.rb -b branch
```

### Custom Checklists

If you want to define checklists within your repo, create a ruby file `tools/deploy_complexity/checklists.rb`:

```
module Checklists
# Example checklist item
class BlarghDetector < Checklist
def human_name
"Blarg!"
end

def checklist
'- [ ] Removed extraneous blargh'.strip
end

def relevant_for(files)
files.select do |file|
file.ends_with(".rb") && IO.read(file) =~ /blargh/
end
end
end

module_function

# List of checklists to run
def checklists
[BlarghDetector].freeze
end
end
```

And then execute the pr-checklist runner inside of CI using:

```
bundle exec pr-checklist.rb -b branch -c tools/deploy_complexity/checklists.rb
```

### Github Token

pr-checklist.rb requires a github token with role REPO to edit PR descriptions and comment on a PR. Make sure that `GITHUB_TOKEN` is set in the environment.
20 changes: 18 additions & 2 deletions exe/pr-checklist.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

# options and validation
class Options
attr_writer :branch, :token, :org, :repo, :dry_run
attr_writer :branch, :token, :org, :repo, :dry_run, :checklist

def branch
# origin/master or master are both fine, but we need to drop origin/
Expand Down Expand Up @@ -36,6 +36,10 @@ def repo
def dry_run
@dry_run || false
end

def checklist
@checklist || nil
end
end

options = Options.new
Expand Down Expand Up @@ -65,6 +69,11 @@ def dry_run
"-n", "--dry-run",
"Check things, but do not make any edits or comments"
) { |dry_run| options.dry_run = dry_run }

opts.on(
"-c", "--custom-checklist config.rb", String,
"Specify external ruby file to load checklists from"
) { |checklist| options.checklist = checklist }
end.parse!

puts "Checking branch #{options.branch}..."
Expand All @@ -80,7 +89,14 @@ def dry_run

puts "Found pull request #{pr}"
files_changed = `git diff --name-only '#{pr.base}...#{pr.head}'`.split("\n")
checklists = Checklists.for_files(files_changed)

# conditionally load externally defined checklist from project
if options.checklist
puts "Loading external configuration from %s" % [options.checklist]
load options.checklist
end

checklists = Checklists.for_files(Checklists.checklists, files_changed)
new_checklists = pr.update_with_checklists(checklists, options.dry_run)

new_checklists.each do |checklist, files|
Expand Down
162 changes: 11 additions & 151 deletions lib/deploy_complexity/checklists.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,76 +46,6 @@ def relevant_for(files)
end
end

class ElmFactoriesChecklist < Checklist
def human_name
"Elm Factories"
end

def checklist
'
- [ ] Elm fuzz tests: use [shortList](https://github.com/NoRedInk/NoRedInk/blob/72626abf20e44eb339dd60ebb716e9447910127f/ui/tests/SpecHelpers.elm#L59) when a list fuzzer is generating too many cases
'.strip
end

def relevant_for(files)
files.select { |file| file.start_with?("ui/tests/") }
end
end

class CapistranoChecklist < Checklist
def human_name
"Capistrano"
end

def checklist
"
The process for testing capistrano is to deploy the capistrano changes branch to staging prior to merging to master and verify the deploy doesn't explode.

- [ ] Make a branch with capistrano changes
- [ ] Wait for free time to test staging
- [ ] Reset/deploy that branch to staging using the normal jenkins deploy process
- [ ] Verify the deploy passes
- If it doesn't, fix the branch and redeploy until it works
- [ ] If it does, reset back to origin/master and request review of the PR
".strip
end

def relevant_for(files)
files.select do |file|
file == "Capfile" \
|| file == "Gemfile" \
|| file.start_with?("lib/capistrano/") \
|| file.start_with?("lib/deploy/") \
|| file.start_with?("config/deploy") \
|| !file.match('.*[\b_\./]cap[\b_\./].*').nil?
end
end
end

class OpsWorksChecklist < Checklist
def human_name
"OpsWorks"
end

def checklist
"
- [ ] Change the source code branch for staging to the branch being tested in the opsworks UI
- [ ] Rebase your code over `origin/staging` to prevent a successful deploy of your changes from making staging run possibly outdated code
- [ ] Turn on an additional time-based instance in the layer ([see instructions](https://github.com/NoRedInk/wiki/blob/1f618042ed1d6b7c7297ec2672ae568e57944fde/ops-playbook/ops-plays.md#using-opsworks-to-bring-up-an-additional-time-based-instance))
- [ ] Verify that the instances passes setup to online and doesn't fail
".strip
end

def relevant_for(files)
files.select do |file|
file.start_with?("config/deploy") \
|| file.include?("opsworks") \
|| file.start_with?("deploy/") \
|| file.start_with?("lib/deploy/")
end
end
end

class RoutesChecklist < Checklist
def human_name
"Routes"
Expand Down Expand Up @@ -148,73 +78,6 @@ def relevant_for(files)
end
end

class MigrationChecklist < Checklist
def human_name
"Migrations"
end

def checklist
'
- [ ] If there are any potential [Slow Migrations](https://github.com/NoRedInk/wiki/blob/master/Slow-Migrations.md), make sure that:
- [ ] They are in separate PRs so each can be run independently
- [ ] There is a deployment plan where the resulting code on prod will support the db schema both before and after the migration
- [ ] If migrations include dropping a column, modifying a column, or adding a non-nullable column, ensure the previously deployed model is prepared to handle both the previous schema and the new schema. ([See "Rails Migrations with Zero Downtime](https://blog.codeship.com/rails-migrations-zero-downtime/)")
'.strip
end

def relevant_for(files)
files.select { |file| file.start_with? "db/migrate/" }
end
end

class DockerfileChecklist < Checklist
def human_name
"Dockerfile"
end

def checklist
"
- [ ] Dependencies added or changed in the Dockerfile are mirrored in the production/staging/demo environment [Cookbooks](https://github.com/NoRedInk/NoRedInk-opsworks/blob/master/noredink/recipes/setup.rb)
- In case of dependency changes, ensure Capistrano deploys still work:
- [ ] Wait for free time to test staging
- [ ] Reset/deploy that branch to staging using the normal jenkins deploy process
- [ ] Verify the deploy passes
- If it doesn't, fix the branch and redeploy until it works
- [ ] If it does, reset back to origin/master and request review of the PR
".strip
end

def relevant_for(files)
files.select { |file| file.include? "Dockerfile" }
end
end

class NixChecklist < Checklist
def human_name
"Nix"
end

def checklist
'
- [Instructions on how to use Nix](https://github.com/NoRedInk/wiki/blob/master/engineering/using-nix.md)
- [ ] changes build successfully with Nix (`nix-shell --pure` to check)
- [ ] once approved, but before merging, make sure to update the Nix cache so that other people don\'t have to rebuild all changes. Run `script/cache_nix_shell.sh`.
'
end

def relevant_for(files)
files.select do |file|
file.start_with?("nix") \
|| file.end_with?("nix") \
|| file == "Gemfile" \
|| file == "Gemfile.lock" \
|| file.end_with?("package.json") \
|| file.end_with?("package-lock.json") \
|| file == "requirements.txt"
end
end
end

# all done!
# rubocop:enable Style/Documentation
# rubocop:enable Metrics/LineLength
Expand All @@ -236,19 +99,16 @@ def for_files(files)

module_function

CHECKLISTS = [
RubyFactoriesChecklist,
ElmFactoriesChecklist,
CapistranoChecklist,
OpsWorksChecklist,
RoutesChecklist,
ResqueChecklist,
MigrationChecklist,
DockerfileChecklist,
NixChecklist
].freeze

def for_files(files)
Checker.new(CHECKLISTS).for_files(files)
def checklists
[
RubyFactoriesChecklist,
RoutesChecklist,
ResqueChecklist
].freeze
end

def for_files(checklists, files)
puts "Applying %s" % [checklists.join(",").gsub(/Checklists::/, '')]
Checker.new(checklists).for_files(files)
end
end
Loading