-
Notifications
You must be signed in to change notification settings - Fork 521
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
Journal of Learning Analytics #252
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @jooyoungseo !
Thank you for your contribution !
Can you look at the PR template to check every boxes before we go further on the review? https://github.com/rstudio/rticles/blob/master/.github/PULL_REQUEST_TEMPLATE.md
Specifically, there are some points to adjust I think:
- Can you reduce the
sample.bib
to less entry following the PR template recommandation ? - We like to include only required file in the skeleton folder so that it does not disturb the user with some files one don't really need. Do you think that
view.jpg
andresults.pdf
used for the content example of JLA journal (that you put back in the Rmd skeleton) can be removed here ? Maybe by changing the examples ? I think it is possible to build content example without those don't you think ? - In the Rmd file, you left some content part with Latex directly and not converted to markdown syntax ? Is this on purpose ? Do you think it would be interesting to give example of using Markdown syntax instead ?
- We are missing a test for this format.
I'll let you look at this. Thank You.
README.md
Outdated
@@ -39,6 +39,8 @@ The **rticles** package provides a suite of custom [R Markdown](http://rmarkdown | |||
|
|||
- [IEEE Transaction](http://www.ieee.org/publications_standards/publications/authors/author_templates.html) journal submissions | |||
|
|||
- [Journal of Learning Analytics](https://learning-analytics.info/journals/index.php/JLA/about/submissions#authorGuidelines) journal submissions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The link is not working. It may have changed. Can you modify ?
Thanks very much for kindly reviewing this PR, @cderv! I need your advice on my next step. While waiting for its review, I was working on a separate package jladown to add some more capabilities (e.g., bookdown-based multiple formats, etc.). I've almost forgotten this PR until your kind comment (it's been here for a while haha). Do you want me to close this PR as I have a separate package devoted to this journal? Or, do you think it is still worth including its basic template here anyhow? |
I would say it is up to you if you want the template to be included into For bookdown feature, you can use any @yihui what are your thoughts on this ? |
@cderv My thoughts are the same as yours. |
Thanks very much for both of your thoughtful comments! I think it would be great to include this template under rticles anyhow. I will then be able to work on more features on top of this base later on. I will address your (@cderv) feedback soon. :) |
This comment has been minimized.
This comment has been minimized.
JooYoung Seo seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Hello @jooyoungseo, I have updated this PR by merging master branch to add the changes we've made since this PR was started. You can now continue this on proper basis. Are you still planning to finish it ? Thanks! |
@cderv Sorry for not being able to get it done sooner. I don't think I can make it until next year. I will find more spare time to cross the finish line. |
Hello @jooyoungseo Just getting back to you on this for a status update. Is this PR still a project you would like to finish ? Thank you |
Changes
jla_article()
function inarticle.R
.README.md
;DESCRIPTION
; andNEWS.md
.Building
Rd2roxygen::rab()
function to buildrticles
package after change has been made.Testing
I have confirmed that
rticles::jla_article()
output format works.Signed-Form