-
Notifications
You must be signed in to change notification settings - Fork 15
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
Vertex attributes for artifacts #229
Conversation
c93428d
to
2922050
Compare
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 for your additions to this PR @joba00002. Sorry for the late reply, we were very busy during the last weeks.
I had a quick look at your recent changes and spotted a few possible copy-paste errors or statements that need more documentation, etc., please see my individual comments below.
In addition, I checked the list of vertex attributes in #170. Could you please tick all the attributes you already have implemented? From what I have seen now, only two vertex attributes are still missing, right?
- number of comments in an issue (there is an attribute for events already, but maybe someone would like to count the comments only even if all events are configured to be considered for network construction)
- originating mailing list
Please let us know if you have further questions regarding that.
I wanted to tick the ones I have implemented, however, GitHub doesn't seem to allow me to do so. I might not have the necessary permissions to edit that issue. Regarding the originating mailing lists of mail threads, I thought you said in a meeting that the thread ID contains the index of the mailing list as listed in the config file. |
407b18a
to
28f3732
Compare
Add metrics for issues and mail threads. Signed-off-by: Jonathan Baumann <[email protected]>
Includes start and end dates, contributor count and activity count. Signed-off-by: Jonathan Baumann <[email protected]>
Added myself to util-networks-covariates.R and util-data-misc.R Signed-off-by: Jonathan Baumann <[email protected]>
Inserted line breaks for lines that were longer than 120 characters. Signed-off-by: Jonathan Baumann <[email protected]>
Also adjust default names and default values. Signed-off-by: Jonathan Baumann <[email protected]>
Includes date an artifact was added, issue titles, and whether an issue is a pull request. Signed-off-by: Jonathan Baumann <[email protected]>
Fix a broken function, avoid code duplication Signed-off-by: Jonathan Baumann <[email protected]>
Only counts the number of comments, not other issue events. Signed-off-by: Jonathan Baumann <[email protected]>
Also includes artifact last edited date Signed-off-by: Jonathan Baumann <[email protected]>
For issues only, PRs only, and both Signed-off-by: Jonathan Baumann <[email protected]>
Also includes some tests. Signed-off-by: Jonathan Baumann <[email protected]>
6a1bb14
to
2fb72e7
Compare
Old names were confusing, at least this is a consistent pattern Signed-off-by: Jonathan Baumann <[email protected]>
issue.event.count, issue.comment.event.count, and issue.is.pull.request Signed-off-by: Jonathan Baumann <[email protected]>
issue.opened.date, issue.closed.date, issue.last.activity.date, issue.title Signed-off-by: Jonathan Baumann <[email protected]>
As the `issue.state` only displays "open" or "closed", this vertex attribute can additionally determine whether a PR was closed by merging or rejected. Signed-off-by: Jonathan Baumann <[email protected]>
2fb72e7
to
52d40ba
Compare
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.
Today, I've gotten round to review this PR. Thanks for your excellent work on this PR @joba00002!
I could not yet have a look at all of the tests as checking the expected data is very exhausting. I aim at checking the expected results on a sample basis later this week.
Except for the tests, I did a detailed review and spotted a few minor issues that need to be fixed. Please have a closer look at my individual comments. If anything is unclear, please don't hesitate to ask.
incorrect log messages, more comments, reopened merge requests now count as open, and events with multiple lines now only counted once. Signed-off-by: Jonathan Baumann <[email protected]>
While this is not necessary now, it's possible that filtering options may change in the future, making this useful. Signed-off-by: Jonathan Baumann <[email protected]>
Codeface produces thread IDs prefixed with the mailing list ID. Test data now reflects this. Signed-off-by: Jonathan Baumann <[email protected]>
for each mailing list vertex, adds an attribute with the identifier of the corresponding mailing list. Currently, this is just a number, as no further information is provided by codeface. Signed-off-by: Jonathan Baumann <[email protected]>
In the currently used R containers in our CI pipeline, the installation of R package 'systemfonts' (which is a dependency of package 'ggraph') fails as the system library 'libfontconfig1-dev' is not installed within these containers. Hence, install 'libfontconfig1-dev' within the pipeline before executing the test scripts. Signed-off-by: Thomas Bock <[email protected]>
Signed-off-by: Thomas Bock <[email protected]>
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 @joba00002 for all the efforts, looks good to me!
I just spotted a couple of spacing inconsistencies. Please fix them and add your changes to the NEWS.md
, and then this PR is ready to merge.
Spaces around +, newlines before '@return' Signed-off-by: Jonathan Baumann <[email protected]>
98179aa
to
3a4b794
Compare
Signed-off-by: Jonathan Baumann <[email protected]>
3a4b794
to
aac337c
Compare
Addresses #170
Prerequisites
showcase.R
with respect to my changes.dev
.Description
Changelog