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

ASA journal style #111

Merged
merged 3 commits into from
Apr 18, 2017
Merged

ASA journal style #111

merged 3 commits into from
Apr 18, 2017

Conversation

hadley
Copy link
Member

@hadley hadley commented Apr 17, 2017

This looks good from my end. What else do you need for rticles?

@hadley hadley requested a review from jjallaire April 17, 2017 19:20
@hadley hadley changed the title [WIP] First pass at ASA journal style ASA journal style Apr 17, 2017
@jjallaire
Copy link
Member

Could you add a test here (https://github.com/rstudio/rticles/blob/master/tests/testthat/test_formats.R#L42) once this is ready to go. I usually like to run these tests on all platforms before merging b/c the distinct LaTeX environments can easily create incompatibilities.

@hadley
Copy link
Member Author

hadley commented Apr 18, 2017

I refactored test_format() — it didn't work with asa_article because output_file was a full path, rather than a file name. I couldn't quite figure out what you were trying to do, but I think I've captured the intent and simplified the code a little (and all the tests pass for me)

@hadley
Copy link
Member Author

hadley commented Apr 18, 2017

(I also alphabetised the test formats so it's more obvious where you should add your test)

@jjallaire
Copy link
Member

Looks good, thanks!

@jjallaire jjallaire merged commit abe23cd into master Apr 18, 2017
@yihui yihui deleted the asa branch December 6, 2017 20:00
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants