-
Notifications
You must be signed in to change notification settings - Fork 85
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
[gitter] Added gitter dashboard #443
Conversation
Hi @valeriocos, here are the dashboards, I will write documentation and update these dashboards soon as our discussion progresses further. There is a problem I am facing, the dashboards get uploaded successfully to Kibiter, however I get this error, the index pattern |
Thank you for the PR @imnitishng ! WRT the error please:
This should fix the error. Let me know how it goes, thanks! |
Hi @valeriocos, thank you for the quick response. I followed what you said but couldn't get the dashboard. Sorry I forgot to upload the image in the first comment. The indexes I have used for gitter are |
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.
Thank you @imnitishng for the PR. I reviewed the PR together with chaoss/grimoirelab-sirmordred#453.
After applying the changes described in the comments (you may need to delete the .kibana
and execute the task panels again), I was able to see the dashboard (see image below), which looks really nice!
I see that the visualizations use their names as titles. Note that the titles can be tuned (see gif below). This could be a nice contribution to the how to
section in mordred (https://github.com/chaoss/grimoirelab-sirmordred/blob/master/Getting-Started.md#how-to-)
@valeriocos, sure I will make the changes and add documentation here and update the How-to section. Thank you. |
Hi @valeriocos, I have edited the dashboards as you said, I wanted to do this little change but I don't seem to get it. How do I change URLs mentioned in table from plain text to this |
Nice question! You need to update the format of the target attribute, see gif below. Please consider to add your question together with the answer to the How to section of mordred. |
Okay, thanks I will add this in the How-to section too. |
Hi @imnitishng let me know when I can review this PR, thanks |
@valeriocos, it is ready, I updated it. Please have a look. |
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.
Hi @imnitishng , thank you for the PR.
Overall the dashboard looks OK. Find below some suggestions/comments:
- Stats visualization: please reduce the font size
- Pie charts: they could be on the same line, and smaller
- Daily message count and Users reading messages daily could be on the same line
- In the Daily message count viz, replace count with messages in the legend
- Change the label
issues_is_pull
to pull requests - Change the label
issues_is_issue
to issues - Change the label
mentioned.mentioned_username
to mentioned users - The 3 tables could be reduced and put on the same line
- Include a markdown on the top right corner of the dashboard to summarize the purpose of each visualization and include a link to the Sigils documentation (see point below).
- Please avoid empty spaces (as the one on the bottom left corner)
- Document the dashboard (https://github.com/chaoss/grimoirelab-sigils/blob/master/CONTRIBUTING.md#adding-or-updating-documentation). The output should be similar to https://chaoss.github.io/grimoirelab-sigils/panels/docker-smells-and-dependencies/
Thanks for your work @imnitishng ! :)
Hi @valeriocos, sorry for the delay and mistakes. I have fixed them now. Please have a look. |
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.
Thank you @imnitishng for addressing my comments/suggestions. The dashboard looks nice, please check the last refinements and add the documentation:
- The caption of the markdown can be removed.
- The legend for the bar charts Weekly Active Users and Messages sent per hour could be hidden by default.
- The pie charts could include just the first 10 hits, this avoids showing the scroll bar on the right.
- The legend for the bar chart Daily message count could be hidden.
- The legend for the bar charts Users reading messages daily could be completed with
(avg)
and the legend hidden. - Order the tables on count desc order
- Extend the tables to remove the scroll bars on the right.
The final dashboard should look like the one below:
Note that to hide the legends, you have just click the icon >
of the visualization and save the dashboard.
Thank you for your work @imnitishng !
Thanks for the review. I will update the PR ASAP. |
@valeriocos, updated as you said. |
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.
Thank you @imnitishng for addressing the comment. The PR looks great, there are some minor comments to address.
Please double check the dashboard and the image (gitter.png), scroll bars are still visible.
Thanks!
docs/_panels/gitter.md
Outdated
|
||
Each user in a room is uniquely identified by a username and a user-id. This | ||
information is used to identify the most active and mentioned users. | ||
Similarly the links shared are identified as github issues or pull requests |
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.
Missing comma? -> Similarly, the links shared are identified as GitHub
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.
Sure. Will fix.
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.
Thanks!
docs/_panels/gitter.md
Outdated
Each user in a room is uniquely identified by a username and a user-id. This | ||
information is used to identify the most active and mentioned users. | ||
Similarly the links shared are identified as github issues or pull requests | ||
or categorized into URL hostnames if they belong to external sources. |
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.
external or other sources?
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.
Other sources, other websites apart from gitlab or github. (stackoverflow, twitter etc)
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.
ok, thanks for the clarification. I would write it in this way:
.. if they belong to other sources (e.g., StackOverflow, Twitter).
docs/_panels/gitter.md
Outdated
|
||
Weekly, hourly and daily activity of the room is put into perspective by | ||
widgets displaying the number of messages sent or unique users active in | ||
the for selected time frame. |
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.
in the for selected time frame -> in the selected time range (to be consistent with the description below)
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.
Okay thanks, will fix.
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.
Thanks!
7f6b473
to
d889a43
Compare
@valeriocos, I have implemented the changes. Please have a look. We should be good to go now. |
Sorry for the mistake @valeriocos. |
No worries @imnitishng ! It is still not fixed, can you try with the following files below? Please check also the commit message, since everything appears on the same line Thanks |
This commit adds gitter panels visualizations with documentation Signed-off-by: Nitish Gupta <[email protected]>
Fixed it. I hope it's all good now. This PR took longer than expected 😅 |
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.
LGTM, thanks @imnitishng
Added some basic visualizations for gitter backend as discussed here.
Signed-off-by: Nitish Gupta [email protected]