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

Add GO REFs page #513

Merged
merged 6 commits into from
Apr 9, 2024
Merged

Add GO REFs page #513

merged 6 commits into from
Apr 9, 2024

Conversation

pkalita-lbl
Copy link
Contributor

@pkalita-lbl pkalita-lbl commented Mar 7, 2024

Fixes #506

Summary

These changes add a Makefile with a target to fetch the latest gorefs.yaml file from the go-site repo. From this file a single GO REFs page is generated.

Details

  • Add a Makefile with a single target, _data/gorefs.yaml, which fetches the latest gorefs.yaml from the head of the master branch of the go-site repo.
  • Add the _data/gorefs.yaml file to .gitignore. This file will never be checked into this repo to maintain go-site as the source of truth. The Makefile target should be run before doing any development or making a production build.
  • Add a single gorefs.html page which lists all GO REFs. The format of this page is largely based on the existing README file (https://github.com/geneontology/go-site/blob/master/metadata/gorefs/README.md). One notable difference is that this page does not include the "table of contents" at the top of the page. It wasn't clear how much value that really provides. Each GO REF title links to its own page anchor (e.g. /gorefs#GO_REF:0000008).
  • Add an index page at gorefs.html which lists the ID and title of each GO REF. This is modeled on the "table of contents" at the top of https://github.com/geneontology/go-site/blob/master/metadata/gorefs/README.md. Each item links to a separate page for the individual GO_REF. Based on feedback in Get feedback on current new go_refs page go-site#2255, obsolete GO REFs are listed in a separate list at the bottom of the page.
  • Add individual pages for each GO REF (e.g. gorefs/go_ref0000003.html)

Screenshots

Index page:
image

Individual GO REF page:
image

Comments

  • As mentioned, the new page currently only surfaces the same information that the old README file did. In particular the following fields are part of the GO_REF YAML file specification, but not displayed on the page: alt_id, evidence_codes, url. If we want to take this opportunity to display that information we could.
  • You'll notice that plain URLs don't render as links in this page, whereas they did in the README (as displayed on GitHub anyway). This is due to a difference in the Markdown renderers used by GitHub vs Jekyll (further reading here and here). I can keep looking at technical solutions to get them linked. But if all else fails the descriptions may need to be updated to enclose the URLs in angle brackets (<http://example.org>).
    UPDATE: Jekyll's Markdown renderer does not turn plain URLs into links, which are somewhat pervasive in the GO REF descriptions. I added a custom Liquid filter to turn these URLs into links.
  • I talked briefly with @kltm about one long page vs individual pages for GO REFs (or both) and decided to do one long page with anchor links for now. However I'll throw out the standard caveat that anchor links can be a little confusing for sections near the bottom of the page (i.e. when there isn't enough content below the desired section to scroll it all the way to the top of the view).
    UPDATE: After feedback in Get feedback on current new go_refs page go-site#2255 we reversed course and did make an index page and individual pages.

@pkalita-lbl pkalita-lbl requested a review from kltm March 7, 2024 00:52
{% if jekyll.environment == "development" %}
{% unless site.data.gorefs %}
<div class="alert alert-warning">
<strong>Warning!</strong> No GO_REFs found. Did you run <code>make _data/gorefs.yml</code>?
Copy link
Member

Choose a reason for hiding this comment

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

@pkalita-lbl Technically, this should never be an issue as it's handled automatically, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Automatic in the production build and deploy workflow, yes (as outlined in #507). It's not as easy to automate it for a local development environment. I looked into ways to hook into Jekyll's build process to make it fully automatic, full stop, but it felt a bit like overkill. In the end I figured that requiring a developer to run one make command if they want to work on that page isn't a bad compromise.

Copy link
Member

Choose a reason for hiding this comment

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

Makes perfect sense--I just wanted to make sure we weren't going to panic Suzi.

@kltm kltm marked this pull request as ready for review March 7, 2024 01:25
Copy link
Member

@kltm kltm left a comment

Choose a reason for hiding this comment

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

All changes make sense to me.
Can be merged with switchover coordination.

@pkalita-lbl
Copy link
Contributor Author

Thanks for the review. My plan is to wait for the initial feedback from geneontology/go-site#2255 before merging this.

@pkalita-lbl
Copy link
Contributor Author

@kltm I pushed some changes in accordance with geneontology/go-site#2255 and updated the PR description/screenshots. Do you want to give this one last glance before I merge?

@kltm
Copy link
Member

kltm commented Mar 28, 2024

Looking at your changes, I still approve.
@pgaudet If you look at the images above, is this sufficient?

@kltm
Copy link
Member

kltm commented Apr 9, 2024

@pkalita-lbl , talking to @pgaudet , she said that she's good to move ahead.

@pkalita-lbl pkalita-lbl merged commit b375a56 into master Apr 9, 2024
@pkalita-lbl pkalita-lbl deleted the issue-506-gorefs-page branch April 9, 2024 20:51
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.

Pull in a copy of gorefs.yaml from go-site and build pages based on it
2 participants