Fixing test issues and R4 fixes #42
Conversation
|
@tdhock, Can you check if |
|
that is great thanks @lazycipher ! |
|
Thank you! |
|
ok great
…On Thu, Jul 9, 2020 at 10:14 AM Himanshu Singh ***@***.***> wrote:
Thank you!
I'll add a test for #40 <#40>
and then we can merge this to tdhock:r4-fixes then to master.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#42 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAHDX4S3A4SHPOO7UT6RJWLR2X3GXANCNFSM4OVWLU4A>
.
|
|
also make sure to update the NEWS/DESCRIPTION file with a new version number, and what you changed. |
|
Okay! Sure thing! |
|
@tdhock, Can we merge this with tdhock:r4-fixes? |
|
yes please merge. |
| theme.pars <- plot_theme(viz) | ||
| panel_margin_lines <- pt.to.lines(theme.pars$panel.margin) | ||
|
|
||
| if(grid::unitType(theme.pars$panel.margin) == "pt" || grid::unitType(theme.pars$panel.margin) == "points") { |
There was a problem hiding this comment.
why do you need this if????
would be easier to understand (less repetitive) using %in% rather than ||,
if(something %in% c(val1, val2))There was a problem hiding this comment.
Got you! I'll rectify it!
| panel_margin_lines <- pt.to.lines(theme.pars$panel.margin) | ||
|
|
||
| if(grid::unitType(theme.pars$panel.margin) == "pt" || grid::unitType(theme.pars$panel.margin) == "points") { | ||
| test_that("after conversion values unequal", { |
There was a problem hiding this comment.
maybe add a comment to explain why we want to values to be unequal after conversion? not easy to understand the context of where this test comes from ...
Sure thing! I'll update asap! |
R/z_helperFunctions.R
Outdated
| #' Check if character is an RGB hexadecimal color value | ||
| #' @param x character | ||
| #' @return True/False value | ||
| #' @export | ||
| is.rgb <- function(x){ | ||
| if(is.null(x)) { | ||
| TRUE | ||
| } else { | ||
| (grepl("#", x) & nchar(x)==7) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
hey @lazycipher did you edit this is.rgb function at all? I dont see any changes. (did the line endings change or something?)
if there are no significant changes (white space etc) then please do NOT commit, because it makes it more difficult to code review.
There was a problem hiding this comment.
I made changes in the pt.to.lines function.
It looks like there's some issue with the line endings!
I really don't understand why whole file shows as edited while I do change just few lines.
Even on VS Code, It was showing 1 line change before committing.
There was a problem hiding this comment.
you should always review the output of the "git diff" command before you do a "git commit" and make sure to remove any parts of the diff which are not relevant to the intended change you are trying to make.
In terms of editors I would recommend using Emacs for this project because there is good support for both R (via ESS) and JavaScript editing. Or try Rstudio if Emacs is too complicated for you to learn right now (but if you want to become a good programmer you should learn at least one of the two classic editors, emacs and vim)
There was a problem hiding this comment.
I'll make sure to follow this up from now on. I'll use Emacs from now on for this project.
There was a problem hiding this comment.
here are some videos I made about emacs + ess https://www.youtube.com/playlist?list=PLwc48KSH3D1Onsed66FPLywMSIQmAhUYJ
There was a problem hiding this comment.
Thank you so much! I'll go through this.
|
@tdhock, Can I go ahead to merge lazycipher:r4-fixes to tdhock:r4fixes? |
|
yes please merge. |
|
Yes, I used docker for firefox because the needed firefox version and the latest firefox versions had too much difference. Any user who will already be a firefox user would've faced this version issues. So I thought it was best to do. |
Fixed:
All tests passing on forked one: https://travis-ci.org/github/lazycipher/animint2