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

Clr #1

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Clr #1

wants to merge 7 commits into from

Conversation

DrLundRasmussen
Copy link
Collaborator

Major changes: added vignette and editing the readme file.
Minor changes: unit testing of create_seq_bin_part() function.

DrLundRasmussen and others added 5 commits May 13, 2021 08:36
Having some issues with the use_vignette function, so don't think this is a proper vignette... probably just a markdown.
Starting on vignette and editing description
@tystan
Copy link
Owner

tystan commented Jul 2, 2021

Hi @DrLundRasmussen . Looks great and well done. Could I get you to run an "R CMD check" on your branch before I merge? I'll explain this in our meeting if that doesn't make sense. Thanks!

@DrLundRasmussen
Copy link
Collaborator Author

Hi @tystan , thanks!
Tried running a CMD check but don't think I'm doing it correctly. I get the following error:
"[EPERM] Failed to realize '.': operation not permitted"
Would be great if you could explain what I might be doing wrong at our next meeting.

@tystan
Copy link
Owner

tystan commented Jul 9, 2021

Hmmm not sure what's going on @DrLundRasmussen. From a quick search it might be a permissions thing but we'll get to the bottom of it

@DrLundRasmussen
Copy link
Collaborator Author

Hi @tystan!
The CMD check had 2 errors, 3 warnings, and 4 notes.
Enclosed the log.
Noticing some warnings regarding the check of PDF version of manual that doesn't sound too good. Think this relates to the altered readme file, but unsure.

How would you like to proceed...?

00check.log

Copy link
Owner

@tystan tystan 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 all these changes @DrLundRasmussen! I have also looked at the vignette - it looks good and thank you for setting it all up. I just need to work out how to accept the majority of these changes and I'll chat to you about the others

@DrLundRasmussen
Copy link
Collaborator Author

No problem at all @tystan. You've done aaaall the work! Hope it can be of some use but please feel free not to accept any of my changes.

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.

None yet

2 participants