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

[INFRA] add support for building PDF versions of the spec #431

Merged
merged 2 commits into from
Mar 11, 2020
Merged

[INFRA] add support for building PDF versions of the spec #431

merged 2 commits into from
Mar 11, 2020

Conversation

Arshitha
Copy link
Contributor

This is a PR almost identical to PR #400 updated-pdf-version-specs except that this has much cleaner commit history and was created to address the issue of unwanted commits in the above referenced PR.

To gain a better understanding of why certain decisions were made while fixing the issue of generating pdf version of the specs, discussions in the following issues would be useful references:

Bugs Fixed:

  • Stable Table of Contents page
  • Cover page
  • Non-tabular text overflow
  • Overlapping columns of tables (owing to small widths of the pipe tables in the markdown files)
    • Markdown files were edited (one-time edit) to increase the column widths of pipe tables
    • Does not affect the read the docs version of specs. However, if any tables were added to the specs in future, there needs to a standard that works for both pdf AND read the docs version of the specs.
  • Automating header to include the latest release version number and build date (pulled from mkdocs.yml)

Enhancements required (non-exhaustive list):

  • Internal links referencing either the same markdown file or other markdown files break during conversion from markdown to pdf. Currently, all of the internal links are removed and replaced by the accompanying text.
  • Superscripts are lost in conversion as well
  • Tried a couple different family of fonts to fix the missing character warnings for emojis used in the markdowns with no success. It would be ideal to fix this to match the Read The Docs version more closely
  • Entity table in Appendix IV has overlapping unreadable columns since it's markdown equivalent has more than 10 columns. Refer to issue for more discussion

@Arshitha
Copy link
Contributor Author

Arshitha commented Mar 10, 2020

@sappelhoff done! #427 and #430 are redundant at this point and can be confusing. How do we deal with this? Apparently, deleting a PR isn't straightforward?

@sappelhoff
Copy link
Member

@sappelhoff done! #427 and #430 are redundant at this point and can be confusing. How do we deal with this? Apparently, deleting a PR isn't straightforward?

I just closed the PRs, that should suffice.

Copy link
Member

@sappelhoff sappelhoff left a comment

Choose a reason for hiding this comment

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

alright, thanks @Arshitha.

I did a final check and marked three small things. After that I am going to merge this.

@sappelhoff sappelhoff changed the title adding pdf generation files [INFRA] add support for building PDF versions of the spec Mar 11, 2020
Copy link
Member

@sappelhoff sappelhoff left a comment

Choose a reason for hiding this comment

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

Thanks @Arshitha for this big step!

@sappelhoff sappelhoff merged commit dc3e078 into bids-standard:master Mar 11, 2020
@sappelhoff
Copy link
Member

PS: @Arshitha have you already added your contributions to our contributors list? You can just edit this wiki page and upon the next release, we'll add the information from the wiki to the repository:

https://github.com/bids-standard/bids-specification/wiki/Contributors

@Arshitha
Copy link
Contributor Author

@sappelhoff Thank you for all the guidance!

This was referenced Mar 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants