Skip to content
This repository has been archived by the owner on Aug 29, 2022. It is now read-only.

v1 #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

v1 #1

wants to merge 1 commit into from

Conversation

spacechurro
Copy link

No description provided.

Copy link

@ronwsmith ronwsmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks super solid to me. Few comments to consider.

Also, maybe run through rubocop to fix up inconsistent quotes.

@@ -0,0 +1,4 @@
module Land
class ApplicationJob < ActiveJob::Base

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class needed?

@@ -0,0 +1,10 @@
module Land
class Campaign < ApplicationRecord

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove empty line

class EventType < ApplicationRecord
include TableName

lookup_by :event_type, cache: 100

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only this one isn't find_or_create. I'm assuming it's intentional, so maybe we need to add how to add event types to the README or somewhere.

t.create_lookup_table :event_types, small: true
end

execute %[CREATE EXTENSION "uuid-ossp";]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make sure we can use this with Elastic Beanstalk + RDS without too much fuss

@@ -0,0 +1,4 @@
# desc "Explaining what the task does"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File needed?


See README.md before updating this file.

## 0.1.0

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Version number doesn't match VERSION = '0.1.2' later on -- let's sync up or update here?

# frozen_string_literal: true

Rails.application.routes.draw do
# Add your own routes here, or remove this file if you don't have need for it.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File needed?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants