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

Documentation build system improvement (1/3) #211

Closed
wants to merge 5 commits into from
Closed

Documentation build system improvement (1/3) #211

wants to merge 5 commits into from

Conversation

natanlao
Copy link
Member

@natanlao natanlao commented Jan 22, 2019 •
edited by sarpera
Loading

This is pull request 1 of 3:

  • Build system improvements (vaguely specified in Improve doc build system #203) made to develop
  • Build system improvements made to gh-pages
  • Documentation of above changes once the above two PRs are merged

This PR implements the following changes:

  • Reduces footprint of Swagger UI documentation by framing a central, vendored copy of swagger-ui. This reduces per-branch overhead from ~8.0M to ~8.0K, which doesn't seem like that much, but that I think will scale more easily and ideally make builds a little faster.
  • Removed some lingering artifacts and tweaked .gitignore to keep them out of the process
  • Removed a good amount of the Node/npm calls

@natanlao
Reduce Swagger UI footprint
4d42c75 
This change should reduce repo footprint total by
using a frame to produce the Swagger UI readout
from the central, vendored copy of swagger-ui.
@natanlao natanlao changed the title Documentation system build improvement (1/3) Documentation build system improvement (1/3) Jan 22, 2019
@natanlao natanlao force-pushed the issues/203-doc-build-improvements branch from 530db06 to a859074 Compare January 22, 2019 17:01
@natanlao
Copy link
Member Author

Also, as I mentioned in the earlier PR, this build system will not function for feature branches until develop is merged into master @briandoconnor

@natanlao natanlao requested a review from jaeddy January 22, 2019 19:59
@natanlao natanlao force-pushed the issues/203-doc-build-improvements branch from a859074 to e7935ab Compare January 23, 2019 02:12
@natanlao natanlao force-pushed the issues/203-doc-build-improvements branch from e7935ab to 2c82766 Compare January 23, 2019 03:14
@briandoconnor
Copy link
Contributor

Hey @jaeddy do you have any bandwidth to look at this before next Monday's call?

@natanlao natanlao mentioned this pull request Jan 31, 2019
Closed
3 tasks
jaeddy
jaeddy approved these changes Feb 11, 2019
View reviewed changes
Copy link
Member

@jaeddy jaeddy left a comment

Choose a reason for hiding this comment

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

Looks great — thanks, @natanlao! Now I just need to update the WES repo to include these improvements...

Makefile

# Make docs available at / for master and at /preview/{branch} for all others.
# We don't need to check if `branch == gh-pages` since Travis will skip it by default.
ifeq "$(BRANCH_NAME)" "master"
Copy link
Member

Choose a reason for hiding this comment

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

For WES, we had agreed to make develop the main "source of truth" for docs, status badges, and GitHub Pages content. While master would be the default branch visible to new visitors (post 1.0), it mostly functions as a passthrough for releases.

I don't think this is worth addressing here, but making a note that we should try to sync up on branch usage, PR procedure, etc. across repos (whether it's following "HubFlow" or something else).

@jaeddy jaeddy mentioned this pull request Feb 11, 2019
Closed
@delagoya
Copy link
Contributor

+1

@denis-yuen
Copy link
Member

FYI @garyluu for dockstore/dockstore#1912

@natanlao
Copy link
Member Author

I have updated documentation that reflects the changes made in this PR. I'll add it to the repo once this is merged, just in case there are any kinks in my PR that I haven't caught yet.

@dglazer
Copy link
Member

dglazer commented Feb 25, 2019

@natanlao , I just merged #231 (after getting @briandoconnor 's approval). You indicated in your approval that you'd need to make some changes to this PR as a result -- flagging so we don't lose track.

@jaeddy
Copy link
Member

jaeddy commented Apr 23, 2019

@briandoconnor / @natanlao — was there any reason that this PR was never merged/closed?

@jaeddy jaeddy mentioned this pull request Apr 28, 2019
Merged
@natanlao natanlao mentioned this pull request May 14, 2019
Open
@natanlao natanlao closed this Oct 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jaeddy jaeddy jaeddy approved these changes

@briandoconnor briandoconnor Awaiting requested review from briandoconnor

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

Successfully merging this pull request may close these issues.
6 participants
@natanlao @briandoconnor @delagoya @denis-yuen @dglazer @jaeddy