search

rudeboybert / fivethirtyeight

R package of data and code behind the stories and interactives at FiveThirtyEight
https://fivethirtyeight-r.netlify.app/
Other
454 stars 104 forks source link

Fixing #55 and #81 #83

Closed rudeboybert closed 4 years ago

rudeboybert commented 4 years ago

Hey @mariumtapal, I'm going to create the PR so that I can easily see side-by-side diffs of all changed files, but I won't review the code for merging until you explicitly tag me for a review (See right-hand menubar)

mariumtapal commented 4 years ago

@rudeboybert I can't tag you for a review since you created this PR!

rudeboybert commented 4 years ago

Also another minor comment @mariumtapal. For all intents and purposes, your above work gets the job done. but whenever possible, it's better to break down your commits into smaller and more modular pieces. For ex in this case one commit for #55 and a separate one for #81. Now that I think about it, you could even argue it might be better to create two separate branches/PRs. That way if you want to revert only one of the two changes, you can do so easily. Let's leave as is this go round