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

Possibly resolves #42 #75

Open
wants to merge 2 commits into
base: devel
Choose a base branch
from
Open

Possibly resolves #42 #75

wants to merge 2 commits into from

Conversation

ObserverHerb
Copy link

@ObserverHerb ObserverHerb commented Nov 30, 2016

Possibly resolves #42.

shows.json can now contain a "limit" member that specifies a character limit for the specific show's title.

Changed hard limit in DataMapper class in suggestion.rb from 40 to the value of live show's limit. Not sure of workflow. May be a problem there is no live show in data.json?
Also bumped setting LIVE_URL to immediately after call to Dotenv.load, otherwise it gets set too late to
initialize the Suggestion class.

…r limit for the specific show's title.

Changed hard limit in DataMapper class in suggestion.rb from 40 to the value of live show's limit. Not sure of workflow. May be a problem there is no live show in data.json?
Also bumped setting LIVE_URL to immediately after call to Dotenv.load, otherwise it gets set too late to
initialize the Suggestion class.

shows.json example:

{
  "shows": [
    {
      "rss":"http://feeds.feedburner.com/BsdNowHd",
      "url": "bn",
      "title": "BSD Now"
    },
    {
      "rss":"http://feeds.feedburner.com/coderradiovideo",
      "url": "cr",
      "title": "Coder Radio",
      "limit": 10
    }
  ]
}
@rikai
Copy link
Owner

rikai commented Dec 3, 2016

I'd like to see this be able to fall back to the hard limit (40) when no limit is specified, i think if that can be done, this can be accepted. 👍

@ObserverHerb
Copy link
Author

It already does. The condition in show.rb...

@limit = json_hash.key?("limit") ? json_hash["limit"] : 40

This pulls the limit key from the json if it exists, otherwise defaults to 40.

@rikai
Copy link
Owner

rikai commented Dec 3, 2016

Whoops, my bad! Totally missed that when i skimmed over the code!

…alid slug, suggestions.rb crashes. Added a condition for when the live show cannot be found so limit defaults to 40.
@ObserverHerb
Copy link
Author

Added stability fix but that now puts the default in two different places as a literal, which will haunt us later.

Do you have a preferred place to set the default limit as a global constant? .env?

@rikai
Copy link
Owner

rikai commented Dec 3, 2016

Generally, anything plugin-related we try to find a way to define in cinchize.yml. .env is usually a last resort location, but is where things should go if it doesn't make sense in cinchize.yml.

@rikai
Copy link
Owner

rikai commented Mar 28, 2017

Any update on this? :)

@ObserverHerb
Copy link
Author

Same as #76.

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.

None yet

2 participants