Skip to content
This repository was archived by the owner on Aug 7, 2018. It is now read-only.

Integrate twitter #21#52

Open
Joerg-Seitz wants to merge 2 commits intomasterfrom
integrate-twitter
Open

Integrate twitter #21#52
Joerg-Seitz wants to merge 2 commits intomasterfrom
integrate-twitter

Conversation

@Joerg-Seitz
Copy link
Collaborator

#21

@codecov-io
Copy link

codecov-io commented Mar 6, 2017

Codecov Report

Merging #52 into master will decrease coverage by 4.9%.
The diff coverage is 51.85%.

@@            Coverage Diff             @@
##           master      #52      +/-   ##
==========================================
- Coverage     100%   95.09%   -4.91%     
==========================================
  Files          22       21       -1     
  Lines         279      265      -14     
==========================================
- Hits          279      252      -27     
- Misses          0       13      +13

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a2a4696...001c560. Read the comment docs.

@@ -0,0 +1,13 @@
class TwitterService
APP_CONFIG = YAML.load(File.read('config/settings/development.yml'))
Copy link
Owner

Choose a reason for hiding this comment

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

Guess you are using this wrong.... the point of using a settings gem is not to load it manually. And especially have it load the right constant for the right environment. Here you hardcode development.yml

@@ -0,0 +1,36 @@
Config.setup do |config|
# Name of the constant exposing loaded settings
config.const_name = 'Settings'
Copy link
Owner

Choose a reason for hiding this comment

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

i guess here you define the const... so you should be able to do something like Settings.some_setting i guess

Copy link
Owner

Choose a reason for hiding this comment

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

also I don't think we need this anymore, since rails ships with a secrets store (the one I linked in the issue)

integrated twitter gem, added development.yaml to .gitignore
@andreasknoepfle
Copy link
Owner

can you rebase onto the latest master ? there are still merge conflicts with this one

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants