git fake data functions git_add, git_commit #232

waylonho · 2023-09-27T12:09:52Z

parameterized git_add and git_commit helper functions

added author and email parameters for git_commit

parameterized git_add and git_commit added author and email parameters for git_commit

carlosparadis

There are some files being modified on accident here for this PR. Please see my inline comments!

carlosparadis · 2023-09-30T07:07:55Z

DESCRIPTION

@@ -43,4 +43,4 @@ Imports:
 VignetteBuilder: knitr
 URL: https://github.com/sailuh/kaiaulu
 BugReports: https://github.com/sailuh/kaiaulu/issues
-RoxygenNote: 7.0.2
+RoxygenNote: 7.2.3


You should not "git add" files which are not needed for the change. This is side effect behavior! This commit should be reverted to not modify this file.

carlosparadis · 2023-09-30T07:08:55Z

R/git.R

+  #' @param folder_path The worktree path
+  #' @param file_to_add The file we want to add
+  #' @export
+  git_add <- function(git_repo, folder_path, file_to_add) {


the function name is already git add, so lets just call the parameter file. Can the filepath be provided rather than just the file name? if so, then we should call it filepath instead!

carlosparadis · 2023-09-30T07:09:14Z

R/git.R

@@ -154,31 +154,51 @@ git_create_sample_log <- function(folder_path="/tmp"){
  git_repo <- file.path(folder_path,'.git')

  # git --git-dir sample/.git --work-tree sample add hello.R
+  #' @param git_repo The git repo path
+  #' @param folder_path The worktree path
+  #' @param file_to_add The file we want to add


Good job on including the docs! 👍

carlosparadis · 2023-09-30T07:10:18Z

conf/geronimo.yml

+    issues: ../rawdata/issue_tracker/geronimo_issues.json
+    issue_comments: ../rawdata/issue_tracker/geronimo_issue_comments.json


This is another file that should not be modified. I also recommend you organize the folders as kaiaulu/kaiaulu so you do not need to keep switching the ../ back and forth every time.

carlosparadis · 2023-09-30T07:10:38Z

man/git_create_sample_log.Rd

Thanks for including the .Rd files!

carlosparadis · 2023-09-30T07:12:23Z

Note your commit did not pass the test coverage checks, although @Rubegen did! You should compare notes on why yours are not passing, and check the log generated above.

I suspect it may be some of the files you may have change on accident.

Please do not delete the fork as you try to change the code! This will kill the Pull Request, and we don't want to create one PR to every attempt :)

This reverts commit 5b5c1a0.

codecov · 2023-10-05T07:29:36Z

Codecov Report

Merging #232 (ce00ea1) into master (3e82c14) will increase coverage by 1.18%.
The diff coverage is 93.40%.

❗ Current head ce00ea1 differs from pull request most recent head a374ccd. Consider uploading reports for the commit a374ccd to get more accurate results

@@            Coverage Diff             @@
##           master     #232      +/-   ##
==========================================
+ Coverage   17.44%   18.62%   +1.18%     
==========================================
  Files          20       20              
  Lines        2534     2572      +38     
==========================================
+ Hits          442      479      +37     
- Misses       2092     2093       +1

Files	Coverage Δ
R/example.R	`100.00% <100.00%> (ø)`
R/git.R	`78.15% <97.56%> (-2.62%)`	⬇️
R/parser.R	`13.30% <44.44%> (+0.34%)`	⬆️

... and 1 file with indirect coverage changes

carlosparadis

Still need a few more changes. Consider using git diff before sending a commit for me to review.

carlosparadis · 2023-10-07T00:37:25Z

vignettes/download_jira_data.Rmd

@@ -49,7 +48,7 @@ save_path_issue_tracker_issue_comments <- conf[["issue_tracker"]][["jira"]][["is

 In the project configuration file, we define the domain which JIRA is hosted. The example configuration file uses the domain for Apache Software Foundation. We can use this information to queue the domain to identify all project keys available in its JIRA:

-```{r eval = FALSE}
+```{r}


You should use git diff before sending the commit so you can spot these. Note you are modifying the original Notebook on parts that should not be modified. This is likely from when I showed how the Notebook works. Regardless, there should be no reason for the Notebook to be added to your commit, since you are not changing that, right?

carlosparadis · 2023-10-07T00:39:36Z

R/git.R

+  git_add(git_repo, folder_path, "hello.R")
+  git_commit(git_repo, folder_path, "This is a commit msg.", "John Smith", "[email protected]")


This is not a script file. It should not execute code outside function definitions.

carlosparadis · 2023-10-07T00:41:59Z

You should also do a git squash at this point to see if the tests goes through.

This reverts commit 0d69ab2.

parameterized git_add and git_commit added author and email parameters for git_commit Revert "i sailuh#227 parameterized git_add, git_commit" This reverts commit 5b5c1a0. i sailuh#227 parameterized git_add, git_commit Revert "i sailuh#227 parameterized git_add, git_commit" This reverts commit 0d69ab2. i sailuh#227 parameterized git_add, git_commit

…/github.com/waylonho/kaiaulu into 227-setup-gitadd-gitcommit-helper-functions

added return(git_repo) for the add and commit functions, trying to pass test

waylonho · 2023-10-13T09:30:51Z

Not sure why this is failing. I've edited the commits so it only changes the git_add and git_commit, and it returns git_repo.

carlosparadis · 2023-10-13T13:10:12Z

If you inspect the log it will say. It is failing a unit test:

Which is:

kaiaulu/tests/testthat/test-parser.R

Lines 52 to 53 in cb2f409

    
           result <- parse_gitlog(perceval_path, git_repo_path) 
        
           expect_is(result, "data.table")

Did you run the unit tests and check command on your local machine?

git_create_sample_log now uses git_init, git_add, and git_commit

waylonho · 2023-10-20T10:33:21Z

Been trying to trouble shoot this for a while. Even when I run the check on the master branch on my local machine, I get the following error:

Looking at the R-CMD-check, this line always brings up the error:

Error ('test-parser.R:52:3'): Calling parse_gitlog with correct perceval and correct git log path returns a data table ──
Error in eval(bysub, x, parent.frame()): object 'data.Author' not found

It is the same error as mentioned on #108. Not sure if this is a file path issue or something else. Any thoughts?

carlosparadis · 2023-10-21T09:50:44Z

Hi,

As we talked about it today, the error you are experiencing on the pull request is likely due to modifications you performed on git_sample_gitlog. Since one of the functions were incorrectly implemented, it is likely git_sample_log is also broken. If the data can't be generated, then parse_gitlog has no data to test, leading to failure.

Executing the unit tests on your local machine will not work, because you have not installed Perceval.

https://github.com/sailuh/kaiaulu/blob/cb2f4098d6aeedb1cf3721a4de75803fe97145f1/.github/workflows/R-CMD-check.yml#L61C11-L61C157

We did the pip on call. What was missing is knowing where the perceval binary was installed in your machine. It should be in the same place as GitHub Actions install it. I.e. it should be on /Library/Frameworks/Python.framework/Versions/Current/bin/perceval

If not, just search for the word "perceval" on your machine.

Please let me know the soonest if you can find this file or not. Thanks!

waylonho · 2023-10-21T14:56:23Z

Hi. I see a perceval file in usr/local/bin and also one in my python3.10/dist-packages folder. Not sure if this is where it's supposed to go. I've attached an image:

Thanks,

carlosparadis · 2023-10-21T15:10:22Z

@waylonho

The thing to keep in mind is that it doesn't matter where the file should be, but rather that you specify its location on tools.yml :) Try to first open the terminal and run ./perceval. See if they work. Also try ./perseval -v and see it is version 0.12.4.

Once you confirm that, get the full path and replace on my tools.yml on Kaiaulu repo with your path. Then try to run parse gitlog function. That should do the trick. Let me know how it goes!

waylonho · 2023-10-22T11:33:37Z

Slightly losing my mind but I will figure this out. I have a way too new version installed. When trying to install 0.12.4, for some reason it's not showing the version. Thought it was peculiar, will look into it again.

carlosparadis · 2023-10-22T11:45:51Z

@waylonho Did we not install 0.12.4 on call? In any case: Try to run Perceval on a .git folder. It does not need to be 0.12.4 to work, but prior group had some issues with more recent versions. If it can parse and generate a table, then you should be fine.

…it_repo) for the add and commit functions running test w/ percevel, only functions have are in git.R. git_create_sample_log calls these functions and returns same git repo. git_commit is commented out and original system2 call is used.

carlosparadis

I didn't exhaustive look at the code on this review, but wanted to point the more immediate PR failed check.

Also, please take a look on CONTRIBUTING.md to go through the checklist, these changes should be added to NEWS.md under documentation sub-section.

Thanks!

carlosparadis · 2023-10-26T05:15:00Z

R/git.R

+#' @param filepath The filepath we want to add
+#' @return The path to the sample .git file.
+#' @export
+git_commit <- function(git_repo, folder_path, commit_msg, author, email) {


Seems the unit tests are now passing, but Check is failing. This seems to be the cause per the log:

carlosparadis · 2023-10-26T05:15:43Z

R/git.R

-  # git --git-dir sample/.git --work-tree sample add hello.R
+  git_add(git_repo, folder_path, file_path)
+
+  # WILL UNCOMMENT THIS BELOW LINE AFTER TEST


Should remove these before sending the Pull Request, or they may be accidentally accepted into the main code.

removed incorrectly defined filepath param, added commit_msg, author, and email in new function for git_commit. Currently the create_sample_gitlog does NOT use the new git_commit function, but rather a hard coded system2 call.

git_checkout function parameter now has boolean, and create_sample_git_log is now calling new git_commit with a fake author

carlosparadis · 2023-11-05T09:57:36Z

@waylonho Unfortunately my edit was ill formatted on the DESCRIPTION file:

* checking DESCRIPTION meta-information ... ERROR
Error: Malformed Authors@R field:
  <text>:14:13: unexpected symbol
13:     person('Waylon', 'Ho', role = c('ctb)),
Error: Error in proc$get_built_file() : Build process failed
14:     person('Nicole
Calls: <Anonymous> ... build_package -> with_envvar -> force -> <Anonymous>
                ^

It is probably missing a comma or a parenthesis somewhere (your commit was missing Nicole's). Since I can't commit directly to your fork, you need to patch this DESCRIPTION file and send another commit so the checks can go through. Let me know if this doesn't make sense.

Signed-off-by: Carlos Paradis <[email protected]>

carlosparadis · 2023-11-05T12:46:25Z

Ok it seems I can commit directly to this branch on your fork. The unit test failing is because of a new test I added tracking another problem. Will commit here in a bit after I fix it on the master branch to see what we get. The description file is now fixed.

…gitcommit-helper-functions Merging updates from upstream

…gitcommit-helper-functions Add commits from Kaiaulu moving to R 4.0.

carlosparadis · 2023-11-05T19:38:03Z

There we go. The checks now pass. I will proceed to code review later today. Make sure you run git pull origin 227-setup-gitadd-gitcommit-helper-functions before you make any changes local, or you wind up unable to edit anything.

Until code review for this is ready, I suggest you, after doing the git pull to this one so you don't forget, focus on the milestone 2.

On your computer, this means you have to 1) git checkout to your master branch, 2) update your master branch with kaiaulu more recent changes:

git remote add upstream [email protected]:sailuh/kaiaulu.git
git pull upstream master
git merge upstream/master master

Resolve any conflicts and commit if needed (it shouldn't, unless you accidentally changed code here).

Then it is business as usual as you do CONTRIBUTING.md: git checkout -b new_branch etc.

carlosparadis

As asked on @Rubegen PR, could you ensure that

the git.R of both your PRs are exact the same
the examples.R only contain your examples
the unit tests only contain your unit tests

I want to avoid reviewing your incomplete git.R functions or the same code twice in case it ended up duplicated. This also should facilitate showcasing your contributions when the PR is accepted.

Thanks!

…mple_log I have included Ruben's updated functions as well as my own. example.R now uses the delete folder function. git_create_sample_log uses the new functions. This commit should contain all the updated functions and finalized git.R and io.R.

waylonho · 2023-11-10T14:37:46Z

Hi, sorry I'm writing this so late. The most recent commit has everything updated:

git.R has all the functions, git_checkout, and git_create_sample_log

io.R has all the functions and is the same as Rubens commit.

I sent Ruben a copy and will update him in the morning.

carlosparadis · 2023-11-10T14:55:21Z

@waylonho

Ruben commit already got merged, so you need to incorporate his code into yours! The simplest way to do this if you don't want to deal with git branches:

Clone the more recent copy of Kaiaulu (not Ruben's) master branch. Add your needed modifications to git.R, io.R, example.R, test-parser.R. Then copy all these files and paste to your branch of this PR. Then send as commit. (Remember to also keep your NEWS.md and DESCRIPTION files.

waylonho · 2023-11-13T03:15:55Z

Hi, I currently have all the files done. Just want to make sure: do you want the PR to include Ruben's recent changes that were merged to master, or just my changes? For example, should example.R has both our tests?

carlosparadis · 2023-11-13T03:22:37Z

@waylonho I see the point of conflict! Since I already merged @Rubegen code, please go ahead and merge everything from master branch to yours, so the conflicts are resolved. The original request was indeed to have your two codes separate, since I was not expecting to have the chance to review either of yours and merge to master. There were a number of other small changes to git.R and io.R so it is easier at this point you just bring your PR up to date so I can merge!

…gitcommit-helper-functions

carlosparadis · 2023-11-13T04:17:12Z

@waylonho

  Functions or methods with usage in documentation object 'example_different_branches' but not in code:
    ‘example_different_branches’
  
  Functions or methods with usage in documentation object 'example_different_files_commits' but not in code:
    ‘example_different_files_commits’
  
  Functions or methods with usage in documentation object 'example_empty_repo' but not in code:
    ‘example_empty_repo’
  
  Functions or methods with usage in documentation object 'git_rename' but not in code:
    ‘git_rename’
  
  Functions or methods with usage in documentation object 'io_create_folder' but not in code:
    ‘io_create_folder’
  
  Functions or methods with usage in documentation object 'io_make_sample_file' but not in code:
    ‘io_make_sample_file’

I suspect the issue are your .Rd files. Try to generate the docs and version them. I am seeing above the old name of functions that I fixed on @Rubegen code. You may want to wait for the checks to finish before sending the next review request just in case too, unless you are stuck!

updated branch from master, changed git_checkout in git.R, example.R, test-parser.R, DESCRIPTION.

fixed function names, testing of checks pass

carlosparadis · 2023-11-13T05:43:26Z

the error generated is because you are missing one function I added to @Rubegen code (Error in io_create_folder("kaiaulu_sample"): could not find function "io_create_folder" I am also a bit worried the NAMESPACE is deleting git mv and re-adding git rename. This is going in the wrong direction :(

As I said before, it may be easier to just to just backup your code, copy all of kaiaulu/master from sailuh git.R, io.R, example.R, and test.R, and then re-add your changes on top of it.

carlosparadis

Very minor changes needed I believe.

Your changes to git.R are reverting the corrections I already did to them on @waylonho code, with I believe the exception of the git_checkout function. So basically, unless you are adding a new function, or improving a new function, can you keep the one I already reviewed on master? Pasting the master functions to your code should fix this, but you need to make sure the example functions are properly using the corrected functions.

carlosparadis · 2023-11-13T05:55:18Z

R/git.R

-
-
-#' Git Init
+#' Initialize a git repo in a folder.


This is reverting the changes on master. Can you leave the code on master as is?

carlosparadis · 2023-11-13T05:55:26Z

R/git.R

 #'
-#' Initializes a new Git repository in the specified folder.
-#' The Git Init command creates a hidden `.git` folder.
+#' This function initializes a new Git repository in the specified folder.


carlosparadis · 2023-11-13T05:55:43Z

R/git.R

  return(git_repo)
 }

-#' Git Commit
+#' Git commit


Also reverting the function from master branch.

carlosparadis · 2023-11-13T05:55:51Z

R/git.R

 #'
-#' Executes the `git commit` command on the target
-#' git repository.
+#' Git commits a file


carlosparadis · 2023-11-13T05:56:01Z

R/git.R

@@ -250,10 +222,10 @@ git_commit <- function(git_repo, folder_path, commit_msg, author, email) {
  return(git_repo)
 }

-#' Git Add
+
+#' Git adds a file to the gitlog sample


Again reverting, etc.

carlosparadis · 2023-11-13T05:56:55Z

R/git.R

 #' @return Any error message generated by git
 #' @export
-git_checkout <- function(commit_hash,git_repo_path){
+git_checkout <- function(commit_hash,git_repo_path,new_branch = FALSE){


the changes to git_checkout look needed. I would keep this on your commit.

waylonho · 2023-11-13T05:59:52Z

Ok, after a few technical difficulties the checks pass now. Everything should be updated and contain Rubens code as well as added changes:

git.R has an updated git_create_sample_log & git_checkout
example.R has all the test cases
test-parser.R has all the unit tests
updated all calls to io functions to proper names (io_make_folder and io_make_file) and added extra folder_path parameter to io_make_folder calls
updated DESCRIPTION, NEWS.md should be the same (Ruben's new feature addition looked sufficient)

And yes, I was attempting to do the clone master and update that branch, but I was running into trouble because github was changing some other files in the 227 branch, other errors, etc. Long story short I updated 227 branch from master, then added to that branch.

carlosparadis · 2023-11-13T06:01:12Z

@waylonho see my review of your passed checks. You're almost there!

waylonho · 2023-11-13T06:05:28Z

@carlosparadis Understood, I'll fix these very shortly.

waylonho · 2023-11-13T07:28:02Z

Hi, I've commited the original function headers from master, the changes should be good to merge. Let me know if there's any other changes.

- supressWarnings were removed from unit tests. - Parameters to example() folder have been added for consistency with other example functions. - The commented unit test that checked empty repo has been "fixed". More specifically, the code causing the issue to the test has been addressed to fail more gracefully returning a value that can be assessed with the unit test (this indirectly adresses sailuh#108). - One of the examples and unit tests purpose was not clear and the filter for large files was missing. This was corrected. - Other small details. Signed-off-by: Carlos Paradis <[email protected]>

carlosparadis · 2023-11-13T15:59:03Z

@waylonho please have a look on a37ccd to see a few things you missed, so you don't repeat on the following PR.

That being said, good work! The merges were properly addressed. There were a few docs inconsistencies and the suppressWarnings that were left in the code, so remember to not use them on the next run! :^)

With this, milestone 1 comes to an end! Thank you for your hard work 👍

i sailuh#227 parameterized git_add, git_commit

5b5c1a0

parameterized git_add and git_commit added author and email parameters for git_commit

carlosparadis requested changes Sep 30, 2023

View reviewed changes

carlosparadis assigned waylonho Sep 30, 2023

Revert "i sailuh#227 parameterized git_add, git_commit"

9a0e089

This reverts commit 5b5c1a0.

i sailuh#227 parameterized git_add, git_commit

0d69ab2

carlosparadis requested changes Oct 7, 2023

View reviewed changes

carlosparadis changed the title ~~parameterized git_add, git_commit~~ git fake data functions git_add, git_commit Oct 7, 2023

carlosparadis mentioned this pull request Oct 7, 2023

Create a fake data generator for git logs #227

Closed

10 tasks

waylonho added 6 commits October 11, 2023 22:10

Revert "i sailuh#227 parameterized git_add, git_commit"

df13bed

This reverts commit 0d69ab2.

i sailuh#227 parameterized git_add, git_commit

c9e27b4

Merge branch '227-setup-gitadd-gitcommit-helper-functions' of https:/…

5f40305

…/github.com/waylonho/kaiaulu into 227-setup-gitadd-gitcommit-helper-functions

checking if checks pass

61401e3

i sailuh#227 parameterized git_add, git_commit

5e5505f

added return(git_repo) for the add and commit functions, trying to pass test

i sailuh#227 edited git_create_sample_log to use parameterized funcs

f3d6628

git_create_sample_log now uses git_init, git_add, and git_commit

carlosparadis requested changes Oct 26, 2023

View reviewed changes

waylonho added 2 commits October 27, 2023 14:33

fixed git_commit params

36bce14

removed incorrectly defined filepath param, added commit_msg, author, and email in new function for git_commit. Currently the create_sample_gitlog does NOT use the new git_commit function, but rather a hard coded system2 call.

added git_checkout, using new git_commit function

905c8d5

git_checkout function parameter now has boolean, and create_sample_git_log is now calling new git_commit with a fake author

carlosparadis added 2 commits November 5, 2023 04:35

Fixes description file

3d372c1

Signed-off-by: Carlos Paradis <[email protected]>

Merge branch 'master' into 227-setup-gitadd-gitcommit-helper-functions

4bdc135

carlosparadis added 2 commits November 5, 2023 05:03

Merge remote-tracking branch 'upstream/master' into 227-setup-gitadd-…

7755ffa

…gitcommit-helper-functions Merging updates from upstream

Merge remote-tracking branch 'upstream/master' into 227-setup-gitadd-…

9e13203

…gitcommit-helper-functions Add commits from Kaiaulu moving to R 4.0.

carlosparadis requested changes Nov 5, 2023

View reviewed changes

carlosparadis mentioned this pull request Nov 10, 2023

git fake data functions, io_create_folder, io_make_sample_file, git_init #233

Merged

waylonho requested a review from carlosparadis November 10, 2023 14:38

Merge remote-tracking branch 'upstream/master' into 227-setup-gitadd-…

d0fd069

…gitcommit-helper-functions

waylonho added 2 commits November 12, 2023 19:23

i sailuh#227 updated branch with master

cab984e

updated branch from master, changed git_checkout in git.R, example.R, test-parser.R, DESCRIPTION.

i sailuh#227 fixed names for git_mv and git_create_sample_log

36ec9ee

fixed function names, testing of checks pass

i sailuh#227 updated function call in git_create_sample_log

0491e6e

carlosparadis requested changes Nov 13, 2023

View reviewed changes

i sailuh#227 fixed git.R function heads to sync with master

5a76b70

carlosparadis merged commit 67ff85f into sailuh:master Nov 13, 2023
2 checks passed

		issues: ../rawdata/issue_tracker/geronimo_issues.json
		issue_comments: ../rawdata/issue_tracker/geronimo_issue_comments.json

		git_add(git_repo, folder_path, "hello.R")
		git_commit(git_repo, folder_path, "This is a commit msg.", "John Smith", "[email protected]")

git fake data functions git_add, git_commit #232

git fake data functions git_add, git_commit #232

Conversation

waylonho commented Sep 27, 2023

carlosparadis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carlosparadis commented Sep 30, 2023

codecov bot commented Oct 5, 2023 • edited Loading

Codecov Report

carlosparadis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carlosparadis commented Oct 7, 2023

waylonho commented Oct 13, 2023

carlosparadis commented Oct 13, 2023

waylonho commented Oct 20, 2023

carlosparadis commented Oct 21, 2023

waylonho commented Oct 21, 2023

carlosparadis commented Oct 21, 2023

waylonho commented Oct 22, 2023

carlosparadis commented Oct 22, 2023

carlosparadis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carlosparadis commented Nov 5, 2023

carlosparadis commented Nov 5, 2023

carlosparadis commented Nov 5, 2023

carlosparadis left a comment

Choose a reason for hiding this comment

waylonho commented Nov 10, 2023

carlosparadis commented Nov 10, 2023

waylonho commented Nov 13, 2023 • edited Loading

carlosparadis commented Nov 13, 2023 • edited Loading

carlosparadis commented Nov 13, 2023

carlosparadis commented Nov 13, 2023 • edited Loading

carlosparadis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

waylonho commented Nov 13, 2023

carlosparadis commented Nov 13, 2023

waylonho commented Nov 13, 2023

waylonho commented Nov 13, 2023

carlosparadis commented Nov 13, 2023

codecov bot commented Oct 5, 2023 •

edited

Loading

waylonho commented Nov 13, 2023 •

edited

Loading

carlosparadis commented Nov 13, 2023 •

edited

Loading

carlosparadis commented Nov 13, 2023 •

edited

Loading