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

Moving documentation that is not specific to phys2bids here #2

Merged
merged 11 commits into from
Dec 2, 2020

Conversation

sangfrois
Copy link
Member

@sangfrois sangfrois commented Nov 13, 2020

docs moved :

  • phys2bids/docs/CODE_OF_CONDUCT.rst became physiopy/docs/community/CODE_OF_CONDUCT.md
  • phys2bids/docs/contributorfile.rsr became physiopy/docs/community/contributorfile.md
  • phys2bids/docs/bestpractices.rst became physiopy/docs/community/best-practices.md

I used pandoc to translate, e.g. while just outside both repo
pandoc git/phys2bids/docs/bestpractice.rst -f rst -t markdown -o git/physiopy/docs/community/best-practices.md

I'm creating an issue in phys2bids so that folks can discuss what should be removed from phys2bids documentation, following this.

Small PRs towards BIG CHANGES

Obviously, this reorganization has to be done in multiple PRs. Let me outline some things I thought about :

  • Include a brief description of the community in docs/index.md

  • Include redirection to each library in libraries/*.md

  • Include versions and releases details under devs/

  • Include a section on electrodermal activity in community/best-practices.md

Also :

I tried running mkdocs serve to run the thing locally and got errors related to my environment. I didn't to clobber it on a friday afternoon (...again) so I'm stopping here.

Copy link
Collaborator

@eurunuela eurunuela left a comment

Choose a reason for hiding this comment

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

LGTM! I wonder if we should expand on the types of contributions in another PR 🤔 the ones we have are quite programming-oriented.

@smoia
Copy link
Member

smoia commented Nov 17, 2020

Question: how can we see the rendered files?

@smoia smoia self-requested a review November 17, 2020 19:11
@smoia smoia added the Documentation This issue or PR is about the documentation label Nov 17, 2020
@sangfrois
Copy link
Member Author

@smoia I am clueless to be honest. First time I work on a GitHub.io website.

I didn't build anything.

@eurunuela
Copy link
Collaborator

I didn't manage to build locally. I got some errors I still have to go through.

@smoia
Copy link
Member

smoia commented Nov 17, 2020

Ok, let me know when you figured it out! I'd like to see it before approving! 😉

@eurunuela
Copy link
Collaborator

Ok, let me know when you figured it out! I'd like to see it before approving! 😉

I managed to run it locally. Apparently, I tried installing the plugins on my own and one of them (the toc-sidebar plugin) has a bug. If you pip install -r requirements.txt you will not have the issue.

Dp mkdocs serve to see live changes on your browser or mkdocs build to build the site locally.

Copy link
Collaborator

@eurunuela eurunuela left a comment

Choose a reason for hiding this comment

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

Hey @sangfrois , I was able to build locally and I see we have some issues.

Basically, we have to point to the correct filenames in mkdocs.yml. I can also see one last reference to niPreps on this same file. We're also missing the contributing.md in the nav.

For contributorfile.md we're gonna have to update the way we write titles for the TOC to work. The titles in the code of conduct are correct, you can use them as a reference.

We're missing the best practices page in the nav too.

We should probably remove the New features page.

@sangfrois
Copy link
Member Author

sangfrois commented Nov 20, 2020

  • mkdocs.yml points to correct path
  • contributing.md is in navbar
  • removed docs/libraries/singularity.md
  • removed New Features page

I did not know what you meant by changing the titles in contributorfile.md @eurunuela ; Do you mean the links embedded in the titles ?

Also, I can't build it eventhough I have all the dependencies. So I'm kind of blind here.
AttributeError: module 'mkdocs.utils' has no attribute 'string_types'

@eurunuela
Copy link
Collaborator

I did not know what you meant by changing the titles in contributorfile.md @eurunuela ; Do you mean the links embedded in the titles ?

You will need to build the pages to see what I mean. Basically, they do not appear on the table of contents on the right-hand side. My guess is it's related to the Markdown styling you used (maybe coming from the pandoc command).

Also, I can't build it eventhough I have all the dependencies. So I'm kind of blind here.
AttributeError: module 'mkdocs.utils' has no attribute 'string_types'

Did you pip install the requirements.txt file? If you look in to the file, we must use a specific version of mkdocs that has the patch for this error.

@sangfrois
Copy link
Member Author

sangfrois commented Nov 27, 2020

@eurunuela my environment was clobbered and now I'm on a linux machine and everything works fine 💯
I had ran pip install -r requirements.txt and things didn't work on my Windows machine.

Now, I see that links are working properly and Table of Contents are generated automatically. The pandocs command I ran didn't translate links properly so I had to change them manually (from the beggining).

It translated to :
Some title {#some link}

So I had to change it like this :
[Some title](#some link)

@sangfrois
Copy link
Member Author

@smoia I would suggest we merge this and plan with all the physiopy folks (in our next meeting) what to add next. Does that make any sense to you ?

- phys2bids: docs/libraries/phys2bids.md
- phys2denoise: docs/libraries/phys2denoise.md
- peakdet: docs/libraries/peakdet.md
- phys2bids: 'https://phys2bids.readthedocs.io/en/latest/'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wait, I thought we'd have an introductory page with the link to the "real" phys2bids documentation in it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, well I gave it a try. Sorry, i didn't mention that in my comments. I was thinking of suggesting we describe libraries in the Home index.md

Copy link
Member Author

Choose a reason for hiding this comment

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

we can still change it before merging

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, well I gave it a try. Sorry, i didn't mention that in my comments. I was thinking of suggesting we describe libraries in the Home index.md

That's a good option too. I like the idea. Then the tabs will direct to the "real" docs of each package. I like it!

@eurunuela
Copy link
Collaborator

@smoia I would suggest we merge this and plan with all the physiopy folks (in our next meeting) what to add next. Does that make any sense to you ?

I think that's the most sensible step forward right now. Once the structure is set, any contributor can open a PR for a specific page in the docs for example.

@eurunuela
Copy link
Collaborator

@eurunuela my environment was clobbered and now I'm on a linux machine and everything works fine 💯
I had ran pip install -r requirements.txt and things didn't work on my Windows machine.

Now, I see that links are working properly and Table of Contents are generated automatically. The pandocs command I ran didn't translate links properly so I had to change them manually (from the beggining).

It translated to :
Some title {#some link}

So I had to change it like this :
[Some title](#some link)

That's what I thought, something was wrong with the installation. Thank you so much!

@eurunuela
Copy link
Collaborator

@smoia I would suggest we merge this and plan with all the physiopy folks (in our next meeting) what to add next. Does that make any sense to you ?

I think that's the most sensible step forward right now. Once the structure is set, any contributor can open a PR for a specific page in the docs for example.

What's your take on this @smoia ? Shall we merge already?

@vinferrer
Copy link

@eurunuela is there a decision on merging?

@eurunuela
Copy link
Collaborator

eurunuela commented Dec 2, 2020

@eurunuela is there a decision on merging?

I'd merge now... okay, let me make sure it looks right one last time and I'll merge!

Copy link
Collaborator

@eurunuela eurunuela left a comment

Choose a reason for hiding this comment

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

I think once I commit the suggestion we're good to go.

docs/community/contributing.md Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation This issue or PR is about the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants