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

Docs #234

Merged
merged 13 commits into from
Jan 25, 2019
Merged

Docs #234

merged 13 commits into from
Jan 25, 2019

Conversation

ocefpaf
Copy link
Member

@ocefpaf ocefpaf commented Jan 25, 2019

Some problems I found when building the docs:

  • missing image graphics/companylogos.png;
  • documents present but not used: ODM1Services.rst, ODM2Services.rst and changes.rst;
  • several linkcheck failures (run make linkcheck to see the URLs that are failing but many are "dummy" URLs and should be marked as expected failures).

Note that I added an automatic build and upload with doctr but that won't work until we create an ssh encrypted key for the project. (Not sure if I still have the rights to create those here, if not I can guide someone to do it.)

I was able to do it, see https://github.com/ODM2/WOFpy/settings/keys
(My super powers here are still valid ;-p)


Preview:

screenshot from 2019-01-25 15-36-59

@ocefpaf ocefpaf requested a review from emiliom January 25, 2019 17:13
@emiliom
Copy link
Member

emiliom commented Jan 25, 2019

Some problems I found when building the docs:

* missing image `graphics/companylogos.png`;
* documents present but not used: `ODM1Services.rst`, `ODM2Services.rst` and `changes.rst`;
* several `linkcheck` failures (run `make linkcheck` to see the URLs that are failing but many are "dummy" URLs and should be marked as expected failures).

I'm not surprised at all that some things were missing, broken or not used. Once sphinx is working and auto-updating via travis, I'll integrate (or drop) the rst files that are not currently used.

Note that I added an automatic build and upload with doctr

Ah. I'd never heard of https://github.com/drdoctr/doctr. Thanks.

but that won't work until we create an ssh encrypted key for the project. (Not sure if I still have the rights to create those here, if not I can guide someone to do it.)

I got an email notification saying you've added the ssh encrypted key, so I assume that's working now. And yes, you should still have rights, since I haven't removed you and don't plan to 😉. But I'd also like to learn more about what's involved.

Preview:

Thanks!

@emiliom
Copy link
Member

emiliom commented Jan 25, 2019

I was able to do it, see https://github.com/ODM2/WOFpy/settings/keys

👍

(My super powers here are still valid ;-p)

I plan to keep it that way, unless you go to the dark side ...

@emiliom
Copy link
Member

emiliom commented Jan 25, 2019

This PR addresses #233

@ocefpaf
Copy link
Member Author

ocefpaf commented Jan 25, 2019

I got an email notification saying you've added the ssh encrypted key, so I assume that's working now. And yes, you should still have rights, since I haven't removed you and don't plan to 😉. But I'd also like to learn more about what's involved.

Yep. I got that part working. If everything works we should have a GH page with the docs once this is merged. (There may be a few hiccups but I'll only be able to see them when doctr tries to push to gh-pages.)

@ocefpaf
Copy link
Member Author

ocefpaf commented Jan 25, 2019

BTW, I fixed a few test failures too. Nothing major, just updates.

test/test_cli.py Outdated Show resolved Hide resolved
@emiliom
Copy link
Member

emiliom commented Jan 25, 2019

BTW, I fixed a few test failures too. Nothing major, just updates.

Yup, I saw that.

@emiliom
Copy link
Member

emiliom commented Jan 25, 2019

You removed https://github.com/ODM2/WOFpy/blob/master/conda.recipe/meta.yaml. I trust you on that one, since I don't know better. But since that's the only file in the conda.recipe directory, shouldn't we remove the directory as well, or will it be removed automatically?

@ocefpaf
Copy link
Member Author

ocefpaf commented Jan 25, 2019

You removed /conda.recipe/meta.yaml@master. I trust you on that one, since I don't know better. But since that's the only file in the conda.recipe directory, shouldn't we remove the directory as well, or will it be removed automatically?

The directory is also removed (git does not allow for empty directories). The tests are now all on the .appveyor.yml and they don't need conda-build to run.

- conda activate TEST
- python -m pip install . --no-deps -vv
- python wof/examples/flask/cbi/build_cbi_cache.py || exit 1
- pytest -s -rxs -v -k "not test_odm2_dao_sqlite"
Copy link
Member Author

Choose a reason for hiding this comment

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

@emiliom we exchanged 1-line here and all the lines in the conda.recipe for these 5 lines, shorter and easier to follow b/c now the tests are all in a single file.

Copy link
Member

@emiliom emiliom left a comment

Choose a reason for hiding this comment

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

It's good to go.

@emiliom emiliom merged commit 15a1964 into ODM2:master Jan 25, 2019
@ocefpaf ocefpaf mentioned this pull request Jan 25, 2019
@emiliom
Copy link
Member

emiliom commented Jan 25, 2019

It's been 12 minutes since the merge, and http://odm2.github.io/WOFpy/ isn't available yet. How long does it take for that first build? Or is there more you need to do first?

I'm not in a rush. Just want to know what to expect.

@ocefpaf ocefpaf deleted the docs branch January 25, 2019 19:37
@ocefpaf
Copy link
Member Author

ocefpaf commented Jan 25, 2019

It's been 12 minutes since the merge, and http://odm2.github.io/WOFpy isn't available yet. How long does it take for that first build? Or is there more you need to do first?

I'm not in a rush. Just want to know what to expect.

It should not take more than a few minutes, but I forgot to create the doc env. That is fixed in #235

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