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

Use AppVeyor artifact URLs in manuscript front-matter #263

Merged
merged 3 commits into from
Aug 30, 2019
Merged

Use AppVeyor artifact URLs in manuscript front-matter #263

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7d1c827
Update environment on 2019-08-09
dhimmel Aug 9, 2019
1f541a8
Update front-matter to use ci_source.manuscript_url
dhimmel Aug 9, 2019
b07d870
check if ci_source.manuscript_url is defined
dhimmel Aug 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions build/environment.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,27 +7,27 @@ dependencies:
- cffi=1.12.3
- ghp-import=0.5.5
- jinja2=2.10
- jsonschema=3.0.1
- pandas=0.24.2
- pandoc=2.7.2
- jsonschema=3.0.2
- pandas=0.25.0
- pandoc=2.7.3
- pango=1.40.14
- pip=19.1
- psutil=5.6.2
- psutil=5.6.3
- python=3.6.7
- pyyaml=5.1
- requests=2.22.0
- watchdog=0.8.3
- watchdog=0.9.0
- pip:
- errorhandler==2.0.1
- git+https://github.com/manubot/manubot@2a99f4af4a352d2e082988d364e01e2e2809a87b
- git+https://github.com/manubot/manubot@1b885c614634d6e88565ebcf5052b6884132ab32
- jsonref==0.2
- opentimestamps-client==0.6.0
- opentimestamps==0.3.0
- pandoc-eqnos==1.4.3
- pandoc-fignos==1.4.2
- pandoc-tablenos==1.4.2
- pandoc-xnos==1.2.0
- pybase62==0.4.0
- pybase62==0.4.1
- pysha3==1.0.2
- python-bitcoinlib==0.9.0
- requests-cache==0.5.0
Expand Down
2 changes: 1 addition & 1 deletion ci/install.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
set -o errexit \
-o pipefail

wget https://repo.continuum.io/miniconda/Miniconda3-4.6.14-Linux-x86_64.sh \
wget https://repo.continuum.io/miniconda/Miniconda3-4.7.10-Linux-x86_64.sh \
--output-document miniconda.sh
bash miniconda.sh -b -p $HOME/miniconda
source $HOME/miniconda/etc/profile.d/conda.sh
Expand Down
2 changes: 1 addition & 1 deletion content/00.front-matter.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ _A DOI-citable version of this manuscript is available at <https://doi.org/DOI_H
<small><em>
This manuscript
{% if ci_source is defined -%}
Copy link
Member

Choose a reason for hiding this comment

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

Because ci_source.manuscript_url is created in a separate Manubot function add_manuscript_urls_to_ci_params, should we also ensure that ci_source.manuscript_url is defined here? Or only check for ci_source.manuscript_url?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think ci_params will ever exist without manuscript_url with the current manubot software because:

https://github.com/manubot/manubot/blob/1b885c614634d6e88565ebcf5052b6884132ab32/manubot/process/util.py#L193-L196

But it doesn't hurt to be safe and possibly some users will have an outdated manubot python package with this version of content/00.front-matter.md , so I added the extra check in b07d870

([permalink](https://{{ci_source.repo_owner}}.github.io/{{ci_source.repo_name}}/v/{{ci_source.commit}}/))
([permalink]({{ci_source.manuscript_url}}))
{% endif -%}
was automatically generated
{% if ci_source is defined -%}
Copy link
Member

Choose a reason for hiding this comment

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

It's not obvious to me which commit we should show in line 19 below for AppVeyor builds. The manuscript filename contains the triggering commit, but this link contains the merge commit for a pull request build. However, the triggering commit doesn't correspond to the ci_source.repo_slug tree. It resides in the tree of the forked repository.

I don't see a better option that what is already here. I do expect the difference between the filename and link could cause confusion.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes it is confusing, but I think it's important that the manuscript contains the commit that actually created it.

Expand Down