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

Journal of Learning Analytics #252

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

jooyoungseo
Copy link

Changes

  • Added Journal of Learning Analytics LaTeX template according to the code of conduct.
  • Added a jla_article() function in article.R.
  • Added related info to README.md; DESCRIPTION; and NEWS.md.

Building

  • Used Rd2roxygen::rab() function to build rticles package after change has been made.

Testing

I have confirmed that rticles::jla_article() output format works.

Signed-Form

  • I have signed and submitted the RStudio form to J.J.

Copy link
Collaborator

@cderv cderv left a 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 and results.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
Copy link
Collaborator

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 ?

@jooyoungseo
Copy link
Author

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?

@cderv
Copy link
Collaborator

cderv commented Aug 6, 2020

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 rticles which is already on CRAN. We provide only pdf format at the moment I think. I see you worked on the Word template too ?

For bookdown feature, you can use any rticles format in bookdown::pdf_book by providing the format into base_format argument.

@yihui what are your thoughts on this ?

@yihui
Copy link
Member

yihui commented Aug 6, 2020

@cderv My thoughts are the same as yours.

@jooyoungseo
Copy link
Author

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. :)

@cderv cderv added this to the v0.16 milestone Aug 11, 2020
@cderv

This comment has been minimized.

@cderv cderv removed this from the v0.16 milestone Aug 12, 2020
@cderv cderv removed their assignment Sep 7, 2021
@cderv cderv marked this pull request as draft September 7, 2021 14:44
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ cderv
❌ JooYoung Seo


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.

@cderv
Copy link
Collaborator

cderv commented Nov 30, 2021

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!

@jooyoungseo
Copy link
Author

@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.

@cderv
Copy link
Collaborator

cderv commented Apr 19, 2023

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

@cderv cderv added the WIP label Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants