Skip to content
This repository has been archived by the owner on Feb 22, 2020. It is now read-only.

#15 - Add links to the project structure documentation #83

Merged
merged 1 commit into from Sep 11, 2017

Conversation

eltonlaw
Copy link
Contributor

Description

(resubmission of PR#64 - couldn't just reopen it cause I deleted the old fork and there was some weird merging thing going on)

Hi, I've added links for everything below. There weren't any explicit instructions on where to link so I just linked to the documentation home for the most part. On some occasions, I would link to a specific part of the documentation (Django Migrations, Docker Containers etc.):

  • Docker
  • docker-compose
  • sphinx
  • Django
  • Django Rest Framework
  • Vue.js
  • Django database migrations
  • requirements.txt
  • DICOM
  • git-lfs
  • Flask
  • flake8

Links not added (there didn't seem to be any place to put them, advice?):

  • pip
  • pytest

Oh yea, while I was looking through it I found some minor typos so I also added those changes.

Reference to official issue

#15 - Add links to the project structure documentation

Motivation and Context

How Has This Been Tested?

I tested with make html and it built with no error. I also ran it and went through and clicked on each link.

Screenshots (if appropriate):

CLA

  • I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

@isms isms requested a review from reubano August 29, 2017 20:11
@reubano
Copy link
Contributor

reubano commented Sep 1, 2017

Looks like the style tag isn't getting any love :). Any chance you can see how things look without it?

<style> tr:nth-child(odd) { background-color: #f6f6f6; } table#structure-table { border-collapse: separate; border-spacing: 0 5px; } </style>

screen shot 2017-09-01 at 12 51 22 pm

@lamby
Copy link
Contributor

lamby commented Sep 3, 2017

Hey @eltonlaw Anything blocking on you on this? :)

@eltonlaw
Copy link
Contributor Author

eltonlaw commented Sep 3, 2017

@lamby Yeah, I can't seem to run make anymore without django throwing an error. autodoc can't import stuff from backend because the django settings are improperly configured, it says that I need to define the environment variable DJANGO_SETTINGS_MODULE. Any ideas on how to fix this or bypass this, since I really only need to render the one file.

If this seems like a headache to deal with feel free to pass this issue to someone else.

@lamby
Copy link
Contributor

lamby commented Sep 4, 2017

@eltonlaw

If you are having a specifc problem please provide full tracebacks and full error messages etc to a pastebin - the messages you are seeing are somewhat generic. However, are you sure you have created a virtualenv, followed the project setup docs, etc. etc.? :)

@eltonlaw
Copy link
Contributor Author

eltonlaw commented Sep 4, 2017

Whoops sorry about that, here's the error message

Yeah I think I did everything:

  • Created a virtualenv and activated it
  • pip install -r requirements.txt
  • docker-compose -f local.yml build
  • docker-compose -f local.yml up.
  • cd docs/
  • make html

I've tried cloning a fresh repo and running the above but I still get an error, it's a slightly different error message: [Error Message | Traceback]

Oh by the way random note, while messing around with the installation process I get

Double requirement given: sphinx_rtd_theme==0.2.4 (from -r requirements.txt (line 7)) (already in sphinx_rtd_theme==0.1.9 (from -r prediction/requirements/local.txt (line 6)), name='sphinx-rtd-theme')

Is this on purpose?

@lamby
Copy link
Contributor

lamby commented Sep 5, 2017

@eltonlaw

Did you try DJANGO_SETTINGS_MODULE=config.settings.local make html

Oh by the way random note, while messing around with the installation process I get "Double requirement given"

Please file as a new issue :)

@lamby
Copy link
Contributor

lamby commented Sep 5, 2017

Please also rename this pull request ;)

@eltonlaw eltonlaw changed the title resubmit to solve #15 #15 - Add links to the project structure documentation Sep 5, 2017
@eltonlaw
Copy link
Contributor Author

eltonlaw commented Sep 5, 2017

@lamby I tried out your suggestion, it didn't work either. But I fixed it in a hacky way, I just deleted apidoc_interface and it builds now with make html, so there's that. Sorry I really should have thought of it earlier.

Oh by the way, here's the error message when running DJANGO_SETTINGS_MODULE=config.settings.local make html if you want to debug it: https://pastebin.com/wxXHwJpB

@reubano Sorry for the delay here's a screenshot of how it renders without the CSS. It looks less clean imo, if the only problem is that the <style> stuff renders as text on GitHub do you want to throw it all into _static/c2c_custom.css adding id="structure-tr" to all tags and with the following names?

tr.structure-tr:nth-child(odd) {
    ...
}
table#structure-table {
    ...
}

screen shot 2017-09-05 at 5 46 08 pm

@reubano
Copy link
Contributor

reubano commented Sep 6, 2017

if the only problem is that the <style> stuff renders as text on GitHub...

Yea, I think moving the style tags to the css file is a great idea! Can you give it a try?

@eltonlaw
Copy link
Contributor Author

eltonlaw commented Sep 6, 2017

@reubano For verification, here's how it looks running make, everything should be just as is, is this okay to merge?

screen shot 2017-09-06 at 7 00 57 pm

@reubano
Copy link
Contributor

reubano commented Sep 7, 2017

Looks good. Do you mind squashing to 1 commit?

@eltonlaw
Copy link
Contributor Author

eltonlaw commented Sep 7, 2017

Uhh, how's that, I don't know if I did it correctly.

@reubano
Copy link
Contributor

reubano commented Sep 7, 2017

Uhh, how's that, I don't know if I did it correctly.

Almost! You got the squashing down, but the commit message needs a little tweaking. You can try something like this: git commit --amend -m "Add links to the project structure documentation"

Squashed commit of the following:

commit abf2114
Author: EltonLaw <eltonlaw296@gmail.com>
Date:   Wed Sep 6 18:57:18 2017 -0400

    move internal stylesheet to _static/c2c_custom.css

commit 5549747
Author: EltonLaw <eltonlaw296@gmail.com>
Date:   Tue Aug 29 12:57:00 2017 -0400

    resubmit to solve drivendataorg#15
@eltonlaw
Copy link
Contributor Author

eltonlaw commented Sep 8, 2017

sweet that worked great thanks, btw I left the rest of the old commit message in the body

@lamby
Copy link
Contributor

lamby commented Sep 9, 2017

@reubano LGTM. Not merging as assigned to thee. :)

@reubano reubano merged commit 208eea5 into drivendataorg:master Sep 11, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants