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.

Sign up for GitHub

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

Jump to bottom

Improvements #9

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Improvements #9

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
8274bc3
Change birthdays database to YAML / JSON
shinenelson May 28, 2017
17a3ea0
Configurize db_path for the birthdays database
shinenelson May 28, 2017
726d61a
Rename configurations.json to configuration.json.example
shinenelson May 28, 2017
c45e1e1
Optimize bot to work with {hash}ed birthdays
shinenelson May 28, 2017
9f7bc88
Configurize mention'ing team members
shinenelson Jun 4, 2017
dfbcc78
Update README.md with more detailed instructions
shinenelson Jun 4, 2017
d409122
Rename db_path to birthdays_path (Resolves https://github.com/jeKnowl…
shinenelson Jun 4, 2017
077a104
Log Time.now
shinenelson Jun 4, 2017
2ee35b3
change date keys to string
shinenelson Aug 1, 2017
9b83de9
handle empty / nil YAML.load_file
shinenelson Aug 1, 2017
ed5c9cb
remove spaces (Resolves https://github.com/jeKnowledge/slack-birthday…
shinenelson Aug 3, 2017
e901b71
move birthday list filter to birthday_bot from birthday_reader
shinenelson Aug 7, 2017
f4dbfe8
add today = Time.now (Resolves https://github.com/jeKnowledge/slack-b…
shinenelson Aug 7, 2017
e3a0763
def build_user_list
shinenelson Aug 7, 2017
56c8b07
[refactor] remove httparty dependency
shinenelson Nov 21, 2019
e64afd9
[docs] replace db_path with birthdays_path
shinenelson Nov 21, 2019
0596516
[readme] fix URL in contributors' list
shinenelson Nov 21, 2019
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
1 change: 0 additions & 1 deletion birthdays.example.txt
View file
Open in desktop

This file was deleted.

17 changes: 17 additions & 0 deletions birthdays.json.example
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
01 :
{
01 : [ "elvina.slovak" ]
},

02 :
{
02 : [ "john.doe", "jane.doe" ],
28 : [ "chris.nolan" ],
},

04 :
{
15 : [ "erwin.down", "leora.pontious", "randy.young" ]
}
}
9 changes: 9 additions & 0 deletions birthdays.yaml.example
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
01 :
01 : [ "elvina.slovak" ]

02 :
02 : [ "john.doe", "jane.doe" ]
28 : [ "chris.nolan" ]

04
15 : [ "erwin.down", "leora.pontious", "randy.young" ]
9 changes: 4 additions & 5 deletions lib/birthday_reader.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
require 'yaml'

class BirthdayReader
def self.get_birthdays(file_path)
birthdays = []
if File.exist?(file_path)
file = File.new(file_path, 'r')
file.each_line do |line|
birthdays << line.chomp.split
end
file.close
bday_list = YAML.load_file(file_path)
Copy link
Collaborator

Choose a reason for hiding this comment

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

The purpose of this class is to read birthdays only, I don't think we should return only the birthdays according to Time.now, instead we should return all of them.

Copy link
Author

Choose a reason for hiding this comment

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

The primary reason I switched to a structured format for the database was to drill down to the exact date to fetch all the birthdays on the day. What would be the use case to have return all the birthdays together? It would just use more memory (for no particular gain, whatsoever) IMHO.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not against the structuring.

What I am saying is that the purpose of the function is to read birthdays. It is not to read and filter. That is why I think we should not filter the birthdays here.

And because we are just managing strings of birthdays I guess memory is not a concern.

birthdays = bday_list[Time.now.month][Time.now.day]
end
return birthdays
end
Expand Down