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

Remove function 'get.commit.data' #163

Merged
merged 5 commits into from
Apr 17, 2019
Merged

Remove function 'get.commit.data' #163

merged 5 commits into from
Apr 17, 2019

Conversation

jkronaw
Copy link
Contributor

@jkronaw jkronaw commented Apr 17, 2019

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.
  • The pull request is opened against the branch dev.

Description

This pull removes the function get.commit.data and replaces calls to this function with simpler statements of equivalent functionality despite that they now use the function get.commits.filtered to retrieve the filtered commits instead of get.commits which was previously used within the by now deleted function get.commit.data.

Changelog

  • Remove the two functions get.author.class.activity and get.author.class.activity.overview from the file util-core-peripheral.R (61b344a)
  • Remove function get.commit.data from util-data.R and replace all calls to this function with statements of equivalent functionality despite the fact that they are now retrieving the commit data via get.commits.filtered instead of get.commits which was internally used in the function get.commit.data (486c105, 9194dfc, e259522, Update core-peripheral module #70)

…rview'

Remove the two functions 'get.author.class.activity' and
'get.author.class.activity.overview' from the file 'util-core-peripheral.R'.

Signed-off-by: Jakob Kronawitter <kronawij@fim.uni-passau.de>
Copy link
Collaborator

@bockthom bockthom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this pull request @SCPhantom . This helps a lot to "clean up" the core-peripheral module, looks good now. I only found some minor issues, please have a look at them.

(Disclaimer: I did not have a look at the changes to the tests yet.)

tests/test-networks-covariates.R Outdated Show resolved Hide resolved
util-core-peripheral.R Outdated Show resolved Hide resolved
@clhunsen clhunsen added this to the v3.5 milestone Apr 17, 2019
Replace 'get.commits' call with a 'get.commits.filtered' call in the function
'get.commit.data' and adjust affected test cases accordingly.

This change was made because at each place where 'get.commit.data' is used, it
is not wanted that successive calculations work with unfiltered commits.

This relates to issue #70.

Signed-off-by: Jakob Kronawitter <kronawij@fim.uni-passau.de>
In preparation to the removal of the function 'get.commit.data', remove all
calls to this function and replace them with equivalent statements that use
the function 'get.commits.filtered'.

This relates to issue #70.

Signed-off-by: Jakob Kronawitter <kronawij@fim.uni-passau.de>
Remove the function 'get.commit.data' from the file 'util-data.R'.

This relates to issue #70.

Signed-off-by: Jakob Kronawitter <kronawij@fim.uni-passau.de>
Signed-off-by: Jakob Kronawitter <kronawij@fim.uni-passau.de>
@jkronaw
Copy link
Contributor Author

jkronaw commented Apr 17, 2019

I have fixed the two issues in a rebase since this PR is rather small anyways

Copy link
Collaborator

@clhunsen clhunsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thanks for the changes, @SCPhantom!

@clhunsen clhunsen merged commit 05dda9d into se-sic:dev Apr 17, 2019
@bockthom bockthom mentioned this pull request Jul 26, 2021
10 tasks
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.

3 participants