bedatadriven / activityinfo-R

ActivityInfo R Language Client
https://www.activityinfo.org/support/docs/R/
17 stars 12 forks source link

Importer improvements #59

Closed akbertram closed 1 year ago

akbertram commented 1 year ago

A number of small fixes and refactoring for the importer.

akbertram commented 1 year ago

@nickdickinson is there a way to get the checks running on pull requests?

nickdickinson commented 1 year ago

It is possible to do. Right now it will for pull requests to only 'main' and 'master' but I believe if we remove that specification from tic.yml it will run on any pull request. Shall I do that?

akbertram commented 1 year ago

Sounds good!

nickdickinson commented 1 year ago

Great, I'll make the change to 4.32 so when we merge it to main in the next release it will start in earnest. For now, I've run a job manually against the importer-improvements branch.

nickdickinson commented 1 year ago

Hi Alex, the workflow failed because of a warning about the parentIdColumn argument not being added to the Roxygen: https://github.com/bedatadriven/activityinfo-R/actions/runs/4032578835/jobs/6932550069

❯ checking Rd \usage sections ... WARNING Undocumented arguments in documentation object 'importTable' ‘parentIdColumn’

codecov-commenter commented 1 year ago

Codecov Report

Base: 43.84% // Head: 41.15% // Decreases project coverage by -2.70% :warning:

Coverage data is based on head (73c2515) compared to base (a92dcf6). Patch coverage: 0.00% of modified lines in pull request are covered.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files ```diff @@ Coverage Diff @@ ## version-4.32 #59 +/- ## ================================================ - Coverage 43.84% 41.15% -2.70% ================================================ Files 12 12 Lines 764 814 +50 ================================================ Hits 335 335 - Misses 429 479 +50 ``` | [Impacted Files](https://codecov.io/gh/bedatadriven/activityinfo-R/pull/59?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BeDataDriven) | Coverage Δ | | |---|---|---| | [R/extractLong.R](https://codecov.io/gh/bedatadriven/activityinfo-R/pull/59?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BeDataDriven#diff-Ui9leHRyYWN0TG9uZy5S) | `0.00% <0.00%> (ø)` | | | [R/import.R](https://codecov.io/gh/bedatadriven/activityinfo-R/pull/59?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BeDataDriven#diff-Ui9pbXBvcnQuUg==) | `0.00% <0.00%> (ø)` | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BeDataDriven). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BeDataDriven)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

nickdickinson commented 1 year ago

Hi Alex, there is one conflicting file in this branch on line 88 in import.R:

<<<<<<< importer-improvements
                 paste(head(parentId[!validParentIds]), collapse = ", ")))
=======
                 paste(head(parentIds[!validParentIds]), collapse = ", ")))
>>>>>>> version-4.32
nickdickinson commented 1 year ago

I've resolved the conflict and will proceed to merge after testing.