Fixed Issue #2 #26
Fixed Issue #2 #26
Conversation
| </li> | ||
| <% end %> | ||
| </ul> | ||
| </div> |
There was a problem hiding this comment.
looks like you unindented everything by one space here....
There was a problem hiding this comment.
Thanks for the heads up, spacing should be fixed now.
|
For archives_sidebar.rb But taking amount the +1, will produce an error on line 37, month: month-1, because 0-1= -1 and -1 is not a month. Strangely enough taking out the +1 on line 33 and taking out -1 on line 37, still throws an error and will not display your archive bar. To be honest, I'm not sure why it throws an error as that should work. Modulus 13 resolves the problems of getting a negative month though, also makes it impossible to get 0 as a result. Having month be 0 might throw an error. For _content.html.erb |
| </ul> | ||
| </div> | ||
| <% end %> | ||
| <h3 class="sidebar-title"><%= sidebar.title %></h3> |
There was a problem hiding this comment.
Why are you changing a class name in this PR? Please defend.
There was a problem hiding this comment.
Whoops, that was a fix for Issue 1, I reverted it back to underscores. This commit should only fix Issue 2 and only Issue 2.
|
Exceedingly close, but again, why do you need % at all? Is it necessary? |
|
Removing modulous completely and taking out the plus and minus 1 seems to produce the correct results. However, I'm still uncertain as to why at the moment. It has to do with how does "(entry.month.to_i) " increment itself. If it loops over 1-12, then I think I have the answer. Going to see if I can find a way to check. |
|
I believe "(entry.month.to_i)" only loops through the numbers 1-12. Somewhere in the code, every month has the correct corresponding number to it (January => 1, February => 2 ... December =>12). I entry.month is a string ("1", "2"..."12"), so it gets converted to an integer. "name: I18n.l(Date.new(year, month), format: '%B %Y')" at this point when you put in the right integer I think I18n.l will put in the right month that corresponds to the integer. |
|
Correct. |
| @@ -30,11 +30,11 @@ def parse_request(_contents, _params) | |||
| article_counts = Content.find_by_sql(["select count(*) as count, #{date_func} from contents where type='Article' and published = ? and published_at < ? group by year,month order by year desc,month desc limit ? ", true, Time.now, count.to_i]) | |||
|
|
|||
| @archives = article_counts.map do |entry| | |||
| month = (entry.month.to_i%12)+1 | |||
| month = (entry.month.to_i) | |||
There was a problem hiding this comment.
- Please removing trailing whitespace.
- Parentheses are now unnecessary.
There was a problem hiding this comment.
Added, thanks for the heads up. Didn't even notice the trailing white space.
I think I understand the original logic to it now as well. The original code add +1 to the end of %12 to not get 0. It seems like line 37 month: month-1, was a bit of a red herring. But the point is that you don't even need %12 in the first place and can get rid of these +1 and -1 entirely.
|
Nice work! 👍 |
This branch will
fix #2
Commits show the process of how the bug was fixed.