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.

Sign up for GitHub

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

Jump to bottom

Indicate Corresponding Author / Review Commons Compatibility #470

Merged
merged 5 commits into from
Jul 9, 2022
Merged

Indicate Corresponding Author / Review Commons Compatibility #470

merged 5 commits into from
Jul 9, 2022

Conversation

cgreene
Copy link
Contributor

@cgreene cgreene commented Jul 7, 2022

This PR aims to close #469.

@AppVeyorBot
Copy link

AppVeyor build 1.0.283 for commit 8b1d9ee is now complete.

Found 51 potential spelling error(s). Preview:content/02.delete-me.md:44:adipiscing
content/02.delete-me.md:44:aliqua
content/02.delete-me.md:44:amet
content/02.delete-me.md:44:consectetur
content/02.delete-me.md:44:dolore
content/02.delete-me.md:44:eiusmod
content/02.delete-me.md:44:elit
content/02.delete-me.md:44:incididunt
content/02.delete-me.md:44:ipsum
content/02.delete-me.md:44:labore
content/02.delete-me.md:44:Lorem
content/02.delete-me.md:44:magna
content/02...
The rendered manuscript from this build is temporarily available for download at:

@AppVeyorBot
Copy link

AppVeyor build 1.0.284 for commit 8526cd4 is now complete.

Found 51 potential spelling error(s). Preview:content/02.delete-me.md:44:adipiscing
content/02.delete-me.md:44:aliqua
content/02.delete-me.md:44:amet
content/02.delete-me.md:44:consectetur
content/02.delete-me.md:44:dolore
content/02.delete-me.md:44:eiusmod
content/02.delete-me.md:44:elit
content/02.delete-me.md:44:incididunt
content/02.delete-me.md:44:ipsum
content/02.delete-me.md:44:labore
content/02.delete-me.md:44:Lorem
content/02.delete-me.md:44:magna
content/02...
The rendered manuscript from this build is temporarily available for download at:

@cgreene
Copy link
Contributor Author

cgreene commented Jul 7, 2022

Ok, this build had corresponding set in the metadata.yaml file:
https://ci.appveyor.com/api/buildjobs/ovfe3iu2tyyvgs7n/artifacts/manuscript-1.0.283-8b1d9ee.pdf

This one does not:
https://ci.appveyor.com/api/buildjobs/98vk2kkhgisu57af/artifacts/manuscript-1.0.284-8526cd4.pdf

I didn't find a test suite, but hopefully this can suffice to demonstrate backwards compatibility. I'll add one more commit to put the flag back on in the rootstock so that the demo is complete.

@AppVeyorBot
Copy link

AppVeyor build 1.0.285 for commit bb1177d is now complete.

Found 51 potential spelling error(s). Preview:content/02.delete-me.md:44:adipiscing
content/02.delete-me.md:44:aliqua
content/02.delete-me.md:44:amet
content/02.delete-me.md:44:consectetur
content/02.delete-me.md:44:dolore
content/02.delete-me.md:44:eiusmod
content/02.delete-me.md:44:elit
content/02.delete-me.md:44:incididunt
content/02.delete-me.md:44:ipsum
content/02.delete-me.md:44:labore
content/02.delete-me.md:44:Lorem
content/02.delete-me.md:44:magna
content/02...
The rendered manuscript from this build is temporarily available for download at:

dhimmel
dhimmel reviewed Jul 8, 2022
View reviewed changes
Copy link
Member

@dhimmel dhimmel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. Let's see what format @agitter thinks is best and then either @cgreene or I can update this PR.

content/00.front-matter.md Outdated
@@ -49,3 +49,12 @@ on {{manubot.date}}.
{%- endif %}
</small>
{% endfor %}

{% if manubot.authors|map(attribute='corresponding')|select|first %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, so if no authors have corresponding set to true, the entire block is skipped.

content/00.front-matter.md Outdated
Comment on lines 54 to 59
To whom correspondence should be addressed:
{% for author in manubot.authors %}
{% if author.corresponding is defined and author.corresponding is not none %}
{{author.initials}}: {{author.email}}
{% endif %}
{% endfor %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC the indentation here is causing the corresponding author text to be treated as a code block:

image

If we want a code block, I think the triple backtick syntax is more explicit. But a code block is probably the wrong formatting.

It might be worth looking through some of the existing corresponding author formats manuscripts have used (noted at #469 (comment)) for inspiration on the best format for here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that we probably want some other formatting besides a code block.

agitter
agitter reviewed Jul 9, 2022
View reviewed changes
Copy link
Member

@agitter agitter left a comment •
edited by dhimmel
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I propose a hybrid of what is implemented here and what we had for this version of meta review:

We add a hard-coded symbol next to each corresponding author. Then, we show that symbol in the conditional text block that lists the author contact information.

We should also update the "Manuscript metadata" section of USAGE before merging.

content/00.front-matter.md Outdated
Comment on lines 54 to 59
To whom correspondence should be addressed:
{% for author in manubot.authors %}
{% if author.corresponding is defined and author.corresponding is not none %}
{{author.initials}}: {{author.email}}
{% endif %}
{% endfor %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that we probably want some other formatting besides a code block.

@AppVeyorBot
Copy link

AppVeyor build 1.0.286 for commit 4f4f899 is now complete.

Found 51 potential spelling error(s). Preview:content/02.delete-me.md:44:adipiscing
content/02.delete-me.md:44:aliqua
content/02.delete-me.md:44:amet
content/02.delete-me.md:44:consectetur
content/02.delete-me.md:44:dolore
content/02.delete-me.md:44:eiusmod
content/02.delete-me.md:44:elit
content/02.delete-me.md:44:incididunt
content/02.delete-me.md:44:ipsum
content/02.delete-me.md:44:labore
content/02.delete-me.md:44:Lorem
content/02.delete-me.md:44:magna
content/02...
The rendered manuscript from this build is temporarily available for download at:

@dhimmel
Copy link
Member

dhimmel commented Jul 9, 2022

as of 4f4f899, here is what it looks like with 1, 2, and 0 corresponding authors:

image

image

image

Nice thing is that the names and emails can be copied directly into gmail and it's parsed correctly. Versions built on CI will link to GitHub Issues.

How does this format look? Also considering inline code around each author name + email. Could also use mailto links. I feel like the inline code might be slightly protected against spambots detecting the email address.

@agitter
Copy link
Member

agitter commented Jul 9, 2022

How does this format look?

That format looks very nice. I think it looks better this way than with inline code, and it's unclear to me how much protection those provide against bots.

@dhimmel dhimmel merged commit 9504aa7 into manubot:main Jul 9, 2022
jessegmeyerlab added a commit to jessegmeyerlab/proteomics-tutorial that referenced this pull request Dec 19, 2022
@jessegmeyerlab @cgreene
@dhimmel @ctb @miltondp
Rootstock 2022 12 19 (#57)
cfcf3e2 
* front-matter corresponding author support

merges manubot/rootstock#470
closes manubot/rootstock#469

Co-authored-by: Daniel Himmelstein <[email protected]>

* SETUP.md: remove redundant 'not'

merges manubot/rootstock#475

* front-matter: skip line break when no author ids

merges manubot/rootstock#477

* GH actions: schedule workflow support

merges manubot/rootstock#476

Made possible by recent changes to the GitHub context for schedule workflows in
https://github.com/orgs/community/discussions/12269#discussioncomment-3747667

Commented out by default.

* Update Manubot on 2022-11-12 & date published support

merges manubot/rootstock#478

* Update Manubot on 2022-11-12
* front-matter: conditionally show publication date
   only if it differs from generated date
* GH actions: update action dependencies

* Upgrade environment on 2022-11-22

merges manubot/rootstock#481

Includes Python 3.11 & Pandoc 2.19.2

* Upgrade Manubot to fix pubmed API calls

Upgrades Manubot to latest version on 2022-11-28.

Critical for PubMed citations to get the fix for manubot/manubot#354.

Co-authored-by: Casey Greene <[email protected]>
Co-authored-by: Daniel Himmelstein <[email protected]>
Co-authored-by: C. Titus Brown <[email protected]>
Co-authored-by: Milton Pividori <[email protected]>
danich1 pushed a commit to danich1/word_lapse_manuscript that referenced this pull request Apr 5, 2023
@cgreene @dhimmel @danich1
front-matter corresponding author support
510f3c6 
merges manubot/rootstock#470
closes manubot/rootstock#469

Co-authored-by: Daniel Himmelstein <[email protected]>
ploegieku added a commit to ploegieku/2023-functional-homology-paper that referenced this pull request Aug 6, 2024
@ploegieku @dhimmel
front-matter corresponding author support
2c07a5b 
merges manubot/rootstock#470
closes manubot/rootstock#469

Co-authored-by: Daniel Himmelstein <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dhimmel dhimmel dhimmel left review comments

@agitter agitter agitter left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Review Commons / Template Compatibility
4 participants
@cgreene @AppVeyorBot @dhimmel @agitter