-
Notifications
You must be signed in to change notification settings - Fork 3
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
update sn template to 12/2023 and support csl or natbib #2
update sn template to 12/2023 and support csl or natbib #2
Conversation
Signed-off-by: Cameron Smith <[email protected]>
Signed-off-by: Cameron Smith <[email protected]>
sn-mathphys -> { sn-mathphys-ay , sn-mathphys-num } Signed-off-by: Cameron Smith <[email protected]>
in case it is unclear to users where to set citeproc, natbib, etc Signed-off-by: Cameron Smith <[email protected]>
Signed-off-by: Cameron Smith <[email protected]>
Signed-off-by: Cameron Smith <[email protected]>
Signed-off-by: Cameron Smith <[email protected]>
Hi @cameronraysmith. Thanks for this, I'll review in the next few days! |
- required for compatibility with specifiying a list in `classoption` - https://github.com/quarto-journals/article-format-template/blob/1082930e10855145d390f679121ad0e28b14b625/_extensions/aft/partials/doc-class.tex#L12 Signed-off-by: Cameron Smith <[email protected]>
Signed-off-by: Cameron Smith <[email protected]>
Signed-off-by: Cameron Smith <[email protected]>
Signed-off-by: Cameron Smith <[email protected]>
- sn-mathphys no longer exists Signed-off-by: Cameron Smith <[email protected]>
- vendor csl files for citeproc compatibility Signed-off-by: Cameron Smith <[email protected]>
Signed-off-by: Cameron Smith <[email protected]>
- see `make -n download-csl-files` Signed-off-by: Cameron Smith <[email protected]>
Signed-off-by: Cameron Smith <[email protected]>
Signed-off-by: Cameron Smith <[email protected]>
Signed-off-by: Cameron Smith <[email protected]>
Signed-off-by: Cameron Smith <[email protected]>
Signed-off-by: Cameron Smith <[email protected]>
Signed-off-by: Cameron Smith <[email protected]>
- use natbibstyle rather than journal - update image preview to set size independent of native image size Signed-off-by: Cameron Smith <[email protected]>
f84a4df
to
b94de9a
Compare
Signed-off-by: Cameron Smith <[email protected]>
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: fastforward merge update-122023
If you have the gh cli gh pr checkout 2
git checkout main
git merge update-122023 --ff-only
git push origin main or, if not, then something like 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 ;). 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 quarto-journals/article-format-template#5 (comment) in quarto-journals/article-format-template#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
|
@cameronraysmith thanks a lot for this!! I haven't done anything with that fork, so there's nothing to carry over. |
Thank you @VladimirAlexiev! |
@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. |
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 @cameronraysmith, thank you for the nudge. I lost track of how much time had passed here. Thank you for doing this, it looks mostly great to me. Can you help me understand a few specifics, detailed in the review?
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.
Many thanks for the review @christopherkenny. I'll respond within a week or so. |
Signed-off-by: Cameron Smith <[email protected]>
Signed-off-by: Cameron Smith <[email protected]>
Thanks again for the review @christopherkenny! I won't ping you again unless I don't hear back for more than two weeks. Regarding,
let me know what you think after taking another look at sn-jnl.cls and #2 (comment). 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. |
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.
Thanks for the updates. One just remaining question in the review. I think there's a good option for updating the name to align with other templates.
Also, be sure to add your name to the |
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 |
- christopherkenny#2 (comment) Signed-off-by: Cameron Smith <[email protected]>
Signed-off-by: Cameron Smith <[email protected]>
Signed-off-by: Cameron Smith <[email protected]>
Regarding f1a05ac, 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. |
Signed-off-by: Cameron Smith <[email protected]>
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! |
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 22281b4csl
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 artifactsrenames38b4098 migrates the contents of the journal field tojournal
tonatbibstyle
journal.cite-style
as discussed in update sn template to 12/2023 and support csl or natbib #2 (comment)