Skip to content
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

Add functions to compute a developer's last activity #275

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

bockthom
Copy link
Collaborator

Prerequisites

  • I adhere to the coding conventions (described here) in my code.
  • I have updated the copyright headers of the files I have modified.
  • I have written appropriate commit messages, i.e., I have recorded the goal, the need, the needed changes, and the location of my code modifications for each commit. This includes also, e.g., referencing to relevant issues.
  • I have put signed-off tags in all commits.
  • I have updated the changelog file NEWS.md appropriately.
  • I have checked whether I need to adjust the showcase file showcase.R with respect to my changes.
  • The pull request is opened against the branch dev.

Description

While we already have functionality to compute the first activity per developer, there was no functionality yet to compute the last activity per developer. With this PR, functionality to compute the last activity per developer is added.

Changelog

Added

  • Add function to compute last activity per person and activity type
  • Add function to add vertex attribute "last.activity"

@bockthom bockthom added this to the v4.5 milestone Dec 10, 2024
@bockthom bockthom requested a review from maxloeffler December 10, 2024 18:35
util-networks-covariates.R Outdated Show resolved Hide resolved
While a function to compute the first activity per person and activity type
already existed, this was not the case for the last activity.

Instead of copying the code from first-activity computation, reuse the code by
establishing an additional helper function for aggregating activity dates. This
is used by first and last activity now, and could potentially also be used for
other aggregations (e.g., to compute the mid date of a person's activities).

Signed-off-by: Thomas Bock <[email protected]>
@maxloeffler
Copy link
Contributor

@bockthom I have had a closer look at the changes submitted in this PR and I could not find major issues with it! I only spotted some minor stylistic ones, I will list them below.

@bockthom bockthom force-pushed the thomas-updates branch 2 times, most recently from f4e0214 to b2b32d8 Compare December 17, 2024 12:05
util-networks-covariates.R Outdated Show resolved Hide resolved
util-networks-covariates.R Show resolved Hide resolved
util-networks-covariates.R Outdated Show resolved Hide resolved
Similar to enhancing first-activity computation by last-activity computation in
the previous commit, this can also be done for the corresponding vertex
attributes.

We now have a new function to attribute the "last.activity" vertex attribute.
Both functions to add first and last activity as a vertex attribute make use of
a commonly used helper function that allows to add an aggregated activity date
as a vertex attribute.

Signed-off-by: Thomas Bock <[email protected]>
@maxloeffler
Copy link
Contributor

Thank you @bockthom for correcting the misalignments all over util-networks-covariates.R! I only spotted three tiny misalignments that slipped through (see below). Apart from those, I approve this PR.

@maxloeffler
Copy link
Contributor

The elements in the aggregation.level c(...) are not one space off:

add.vertex.attribute.author.role.function = function(list.of.networks, project.data, classification.function,
name = "author.role",
aggregation.level = c("range", "cumulative", "all.ranges",
"project.cumulative", "project.all.ranges",
"complete"),
default.value = NA) {

Same goes for the elements in the aggregation.level c(...) here:

add.vertex.attribute.author.issue.comment.count = function(list.of.networks, project.data,
name = "issue.comment.count",
aggregation.level = c("range", "cumulative", "all.ranges",
"project.cumulative",
"project.all.ranges", "complete"),
default.value = 0L,
issue.type = c("all", "pull.requests", "issues"),
use.unfiltered.data = FALSE) {

Lastly, these parameters are just overall off:

add.vertex.attribute.artifact.last.edited = function(list.of.networks, project.data, name = "last.edited",
aggregation.level = c("range", "cumulative", "all.ranges",
"project.cumulative",
"project.all.ranges", "complete"),
default.value = NA) {

Note: As you did not edited these lines, it would not let me create a regular review comment and I had to resort to this.

The tests cover the new "last.activity" vertex attribute, which shall implicitly
also cover the last-activity data computation.

Signed-off-by: Thomas Bock <[email protected]>
Signed-off-by: Thomas Bock <[email protected]>
@bockthom
Copy link
Collaborator Author

bockthom commented Jan 12, 2025

I've fixed the three misalignments that you have spotted @maxloeffler (at least, I hope so). If all the misalignments you have spotted are fixed now and if you agree with my descriptions in the NEWS.md, I'd be happy if you could also formally approve this PR 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants