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

Pull requests #260

Merged
merged 20 commits into from
Apr 29, 2024
Merged

Conversation

DylanCarbone
Copy link
Collaborator

Hi @AugustT

Key changes:

  • Tidied the tests of functions using the frescalo executable in the testfrescalo.r file. GHA actions will also run the tests in a windows environment instead of skipping. We need to look into whether frescalo runs in a macos environment. Comments in the old frescalo test file suggests that frescalo does not run on macos, but then I think this would have been raised as an issue.
  • For now I have removed testing in the 'ubuntu-latest' environment which raises errors related to the installation of lme4.
  • Replaced mockery::stub() function with the testthat::with_mocked_bindings() which does not alter global function configurations.

Thanks,

Dylan

@AugustT
Copy link
Member

AugustT commented Apr 29, 2024

@DylanCarbone Why is appveyor still running? I'm happy to merge this, even with the minor lme4 installation bug on the old Ubuntu release, do you agree?

@DylanCarbone
Copy link
Collaborator Author

DylanCarbone commented Apr 29, 2024

@AugustT, that is odd. Please see this issue here. If this is correct, the appveyor failure is actually a log of a previous failure when the appveyor yml existed. The solution they suggest is to commit directly to the repo instead of merging a pull request. This could explain why there is no such issue in the BRCindicators repo, after you made a direct edit to the read me during our last meeting.

Could you please try editing the readme file and pushing directly to the repo?

Thanks

@DylanCarbone
Copy link
Collaborator Author

I'm happy for you to merge, even with the lme4 installation bug. Strangely, that error hasn't occurred in my branch. It must be another random installation error.

@AugustT
Copy link
Member

AugustT commented Apr 29, 2024

Okay I have removed the 'webhooks' for Appveyor which should solve that issue

@AugustT AugustT closed this Apr 29, 2024
@AugustT AugustT reopened this Apr 29, 2024
@AugustT AugustT merged commit 2337f64 into BiologicalRecordsCentre:master Apr 29, 2024
6 of 9 checks passed
@DylanCarbone DylanCarbone deleted the pull_requests branch April 29, 2024 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants