git fake data functions git_add, git_commit

[waylonho]

parameterized git_add and git_commit helper functions

added author and email parameters for git_commit

[carlosparadis]

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 :)

[codecov[bot]]

Codecov Report

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

:exclamation: 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%)	:arrow_down:
R/parser.R	13.30% <44.44%> (+0.34%)	:arrow_up:

... and 1 file with indirect coverage changes

[carlosparadis]

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

[waylonho]

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]

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

Which is:

https://github.com/sailuh/kaiaulu/blob/cb2f4098d6aeedb1cf3721a4de75803fe97145f1/tests/testthat/test-parser.R#L52-L53

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

[waylonho]

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]

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]

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]

@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]

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]

@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.

[carlosparadis]

@waylonho remember to add yourself to the DESCRIPTION file and add a note on NEWS.md too on this PR. See the repo CONTRIBUTING.md for what to edit there. Let me know when this is ready for review by hitting the request for review button on GitHub.

[waylonho]

@carlosparadis I will update the DESCRIPTION and NEWS.md shortly. Everything else works as intended, EXCEPT for this one test case:

https://github.com/sailuh/kaiaulu/blob/90be27ee0a3917d330656203b89e31bf891f4057/tests/testthat/test-parser.R#L73-L82

Specifically, line 78 still brings up the "object 'data.Author' not found" error. For the sake of passing the tests, I commented out the lines. I am unsure of how to fix it and believe it's some way the parse_gitlog is trying to call a table from an empty_repo. I will update the files when I can get the test to pass. Thanks.

Edit: I have updated the description and news.md.

[carlosparadis]

@waylonho Your code is a few commits behind from main branch since I had to update Kaiaulu in between, which is leading to the DESCRIPTION and NEWS.md conflict. To address this, you have to git pull master, then git merge to your changes, and finally commit again to your fork branch associated to this PR.

However, this does not block me from code reviewing. Are the checks passing locally on your machine? Just wanted a final OK before I download your code to review.

[carlosparadis]

Actually let me try and resolve them via GitHub directly. Standby.

[carlosparadis]

@waylonho I commit to your fork to resolve the conflicts. The checks are now able to run on the PR. Please make sure to git pull from your fork to add my resolving conflict commit before you make more code changes, or you will be unable to make further commits to the PR unless you git clone your fork again.

Let's see what the GitHub checks comes up with!

[carlosparadis]

@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.

[carlosparadis]

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.

[carlosparadis]

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 git@github.com: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.

[waylonho]

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]

@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]

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]

@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!

[carlosparadis]

@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!

[carlosparadis]

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.

[waylonho]

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]

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

[waylonho]

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

[waylonho]

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.

[carlosparadis]

@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 👍