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

don't run update_gaia_lite_docs on build #2647

Closed
wants to merge 1 commit into from
Closed

don't run update_gaia_lite_docs on build #2647

wants to merge 1 commit into from

Conversation

zramsay
Copy link
Contributor

@zramsay zramsay commented Oct 31, 2018

this broke some of the Jenkins automation

Note: it isn't clear what this command is meant to do (see documentation in docs/DOCS_README.md)

  • Targeted PR against correct branch (see CONTRIBUTING.md)

  • Linked to github-issue with discussion and accepted design OR link to spec that describes this work.

  • Wrote tests

  • Updated relevant documentation (docs/)

  • Added entries in PENDING.md with issue #

  • rereviewed Files changed in the github PR explorer


For Admin Use:

  • Added appropriate labels to PR (ex. wip, ready-for-review, docs)
  • Reviewers Assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@codecov
Copy link

codecov bot commented Oct 31, 2018

Codecov Report

Merging #2647 into develop will decrease coverage by 0.02%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #2647      +/-   ##
===========================================
- Coverage    58.84%   58.82%   -0.03%     
===========================================
  Files          152      152              
  Lines         9424     9424              
===========================================
- Hits          5546     5544       -2     
- Misses        3508     3510       +2     
  Partials       370      370

@cwgoes
Copy link
Contributor

cwgoes commented Nov 1, 2018

I think inclusion was intentional, this ensures that we update the statically-included Swagger websites from the swagger.yaml file. What's the issue in Jenkins - maybe we need to install the statik binary?

@zramsay
Copy link
Contributor Author

zramsay commented Nov 3, 2018

installing statik on Jenkins (which we're moving off of) is opening a can of worms (doubly so since we're migrating) . Across the whole stack, make build does one thing and all the automation is built to expect that, see #2672

Why not run this target as a pre-commit hook?

@greg-szabo
Copy link
Member

greg-szabo commented Nov 7, 2018

this ensures that we update the statically-included Swagger websites from the swagger.yaml file.

Why is that necessary during the build process?

@cwgoes
Copy link
Contributor

cwgoes commented Nov 7, 2018

Why is that necessary during the build process?

Are we using the output to display the version of Swagger we were statically hosting? If not it doesn't matter.

@zramsay
Copy link
Contributor Author

zramsay commented Nov 7, 2018

replaced by #2709 anyway

@zramsay zramsay closed this Nov 7, 2018
@zramsay zramsay deleted the zach/fix branch November 7, 2018 15:27
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.

3 participants