Closed cameronraysmith closed 4 months ago
Hi @cameronraysmith. Thanks for this, I'll review in the next few days!
Thanks for the template. Sounds good. Found one more issue with using classoption
fixed in 30d9b0f519a6e46e51ac0903188e3d0c2faa7595. Line numbering is turned on by default in 22281b47a26ea3213132a3399c396801e8047432 to illustrate the use of classoption
.
After you review, it would be easier to follow-up with the atomic commit history if you wouldn't mind fastforward-merging from the command line rather than any of the options available from the github web ui. Apologies if this is obvious, but to be clear, for example:
If you have the gh cli ```bash gh pr checkout 2 git checkout main git merge update-122023 --ff-only git push origin main ``` or, if not, then something like ```bash git fetch https://github.com/cameronraysmith/nature.git update-122023 git checkout -b update-122023 FETCH_HEAD git checkout main git merge update-122023 --ff-only git push origin main ``` If this sounds like a crazy request, see https://github.com/orgs/community/discussions/4618 ;). Incidentally, if interested, there's a nice relatively new tool for this: https://github.com/sequoia-pgp/fast-forward . Thank you!
@VladimirAlexiev it'd be great to get your input on this PR regarding what you had planned in your fork or otherwise. Many thanks for your https://github.com/quarto-journals/article-format-template/issues/5#issuecomment-1715545096 in https://github.com/quarto-journals/article-format-template/issues/5, which nicely summarized the current state of work on this template.
This PR does not directly address the points for this fork you mentioned in your comment
No examples of ORCID, colored links, colored text.
@cameronraysmith thanks a lot for this!! I haven't done anything with that fork, so there's nothing to carry over. Regarding ORCID and colored text, maybe you can copy from https://github.com/sebdunnett/quarto-sn ? Cheers!
Thank you @VladimirAlexiev! I suppose simply ensuring consistency with https://github.com/quarto-journals/article-format-template, which I have not done here, will be useful for one additional PR to validate that this template should be relatively straightforward for anyone to use.
@christopherkenny no rush, but also wanted to point out I can also simply host this in my account if you don't want to accept and review broader contributions to your personal copy. I'll assume this is the case if you don't respond.
Many thanks for the review @christopherkenny. I'll respond within a week or so.
Thanks again for the review @christopherkenny! I won't ping you again unless I don't hear back for more than two weeks.
Regarding,
Additionally, open to discussion on the
journal
->natbibstyle
point, but I'm not sure that name improves the understanding, absent another reason to rename it.
let me know what you think after taking another look at sn-jnl.cls and https://github.com/christopherkenny/nature/pull/2#discussion_r1538498117. Just give me a definitive ruling in your next review ;) and I'm happy to update to your liking in what should be a final round.
Otherwise, let me know if there's anything else and please consider my comment about fast-forward merging. Happy to discuss further if the reason for requesting that isn't clear.
Also, be sure to add your name to the _extension.yml
file!
Many thanks for this second review @christopherkenny! Consistency with the upstream templates makes a lot of sense. This one just happens to be the first one I've ever taken a look at. I will update as you suggest within about ~two~three weeks. Feel free to ping me if it takes longer than that.
Also, be sure to add your name to the
_extension.yml
file!
Regarding f1a05ac6690205a895bed65a46f68a09033917f0, it looks like the _extension.yml author field doesn't support a list so I added this as a comma-separated string. Feel free to review if you think something else would be better.
Hi @cameronraysmith. I've reviewed this and I'm happy to merge. Thank you for crafting such a helpful PR! I'll work through your preferred merge.
Looks like that worked, please let me know if you disagree!
Hi @cameronraysmith. I've reviewed this and I'm happy to merge. Thank you for crafting such a helpful PR! I'll work through your preferred merge.
Perfect. Thank you!
This PR
sn-mathphys
intosn-mathphys-ay
andsn-mathphys-num
natbib
instead of the defaultciteproc
cite-method
if it is essential to respect the natbib bibliography styles from the upstream template,
after journal to support use of theclassoption
list now illustrated in 22281b47a26ea3213132a3399c396801e8047432csl
parameter in the template.qmd yaml header and enables html and docx output to illustrate that this approach allows for consistent citation styling among pdf, html, and docx artifactsjournal
tonatbibstyle
~ 38b409864817e00e4f3e3cb158e75b55f857e0cd migrates the contents of the journal field tojournal.cite-style
as discussed in https://github.com/christopherkenny/nature/pull/2#discussion_r1553901528