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

Feature: Persistence #63

Merged
merged 77 commits into from
Jan 8, 2015
Merged

Feature: Persistence #63

merged 77 commits into from
Jan 8, 2015

Conversation

kookster
Copy link
Member

@kookster kookster commented Dec 3, 2014

Handle updating of models and database via API calls.

def tags
(topics + tones + formats + user_tags).map(&:to_tag).uniq.sort
end

def tags=(ts)
self.user_tags = ts.uniq.sort.collect{|t| UserTag.new(name: t) }

Choose a reason for hiding this comment

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

Prefer map over collect.
Space missing to the left of {.
Space between { and | missing.

# check if it should be zoomed, suppress if not
!embed_zoomed?(name, binding[:zoom], options[:zoom])
return !embed_zoomed?(name, binding[:zoom], options[:zoom])

Choose a reason for hiding this comment

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

Redundant return detected.

@@ -7,11 +7,11 @@ class Api::StoriesController < Api::BaseController
filter_resources_by :series_id, :account_id

def resource
@story ||= Story.published.visible.find_by_id(params[:id])
@story ||= params[:id] ? Story.published.visible.find_by_id(params[:id]) : Story.new

Choose a reason for hiding this comment

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

Line is too long. [88/80]

def longest_version
audio_versions.sort_by(&:length).last
end

Choose a reason for hiding this comment

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

Extra empty line detected at body end.

@@ -27,6 +27,10 @@

class ActiveSupport::TestCase
include FactoryGirl::Syntax::Methods

def api_request_opts(opts={})

Choose a reason for hiding this comment

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

Surrounding space missing in default value assignment.

Story.published.visible.find_by_id(params[:id])
else
Story.new
end

Choose a reason for hiding this comment

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

end at 19, 4 is not aligned with if at 15, 15

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.45%) when pulling 19f908e on feat/persist into 2666222 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.45%) when pulling 0f29dff on feat/persist into 2666222 on master.

@kookster
Copy link
Member Author

kookster commented Jan 7, 2015

@chrisrhoden - This should be good to go now, finally.

@cqr
Copy link
Contributor

cqr commented Jan 8, 2015

@kookster looks good, working through merge conflicts now

@kookster
Copy link
Member Author

kookster commented Jan 8, 2015

I can do that

  • Andrew

On Jan 8, 2015, at 12:42 PM, Chris Rhoden notifications@github.com wrote:

@kookster looks good, working through merge conflicts now


Reply to this email directly or view it on GitHub.

Conflicts:
	app/controllers/api/picks_controller.rb
	app/controllers/concerns/hal_actions.rb
	app/representers/api/series_representer.rb
	app/representers/api/story_representer.rb
	app/representers/api/user_representer.rb
	test/controllers/concerns/hal_actions_test.rb
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.46%) when pulling 32438bb on feat/persist into e1ec8ba on master.

@@ -44,7 +44,7 @@ def params
def current_user
FactoryGirl.create(:user)
end

Choose a reason for hiding this comment

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

Trailing whitespace detected.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.46%) when pulling ae61547 on feat/persist into e1ec8ba on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.46%) when pulling 7e9c604 on feat/persist into e1ec8ba on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.46%) when pulling 4c7e5ca on feat/persist into e1ec8ba on master.

cqr added a commit that referenced this pull request Jan 8, 2015
Support writing to the database via the API
@cqr cqr merged commit d9427a2 into master Jan 8, 2015
@cqr cqr deleted the feat/persist branch January 8, 2015 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants