-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Replaced .count with .size in Line 49 #8491
Conversation
Codecov Report
@@ Coverage Diff @@
## main #8491 +/- ##
==========================================
- Coverage 82.01% 81.79% -0.23%
==========================================
Files 101 101
Lines 5894 5894
==========================================
- Hits 4834 4821 -13
- Misses 1060 1073 +13
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job here 💯 almost there
@@ -1,4 +1,4 @@ | |||
<div class="col-md-2"> | |||
m <div class="col-md-2"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like you added this m
by mistake. Kindly edit the file and remove the m
at the beginning of the file and commit.
you can find it here https://github.com/Annysah/plots2/blob/patch-6/app/views/search/all_content.html.erb.
Good job @Annysah, I'm just curious if changing .count to .size also applied to the queries too? Since multiple models use .count.. @cesswairimu can confirm this? |
Also, there are some files that have been left out in the view like and some more.. If you use Visual Studio Code as an editor, you can search the word |
Yeah it applies to some of them, we have other issues opened for some of these files. sometimes |
It's clear now, thanks @cesswairimu |
Code Climate has analyzed commit 8571b01 and detected 0 issues on this pull request. View more on Code Climate. |
Hello @cesswairimu thanks for the review, I have removed the m which was mistakenly added. |
Thanks @RuthNjeri |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Annysah Looking good 🎉 Thanks for working on this!!
Thank you! @Tlazypanda |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome all good now 🎉
Thanks @Annysah for working on this 🚀 would you like to close the other pull requests since we updated all files here? |
Congrats on merging your first pull request! 🙌🎉⚡️ Help others take their first stepNow that you've merged your first pull request, you're the perfect person to help someone else out with this challenging first step. 🙌 Try looking at this list of `first-timers-only` issues, and see if someone else is waiting for feedback, or even stuck! 😕 People often get stuck at the same steps, so you might be able to help someone get unstuck, or help lead them to some documentation that'd help. Reach out and be encouraging and friendly! 😄 🎉 Read about how to help support another newcomer here, or find other ways to offer mutual support here. |
NM, closed them. Thanks everyone |
Oh my!!! I'm so excited that my PR was merged!!! |
Fixes #8489 (<=== Add issue number here)
Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!
rake test
@publiclab/reviewers
for help, in a comment belowIf tests do fail, click on the red
X
to learn why by reading the logs.Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software
Thanks!