-
Notifications
You must be signed in to change notification settings - Fork 103
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
Add Docs for Binder Badge w/GitHub Actions #203
Conversation
Thanks for submitting your first pull request! You are awesome! 🤗 |
@choldgraf stuck on this one, not sure how to resolve the readthedocs error, does anything stick out to you? |
Co-authored-by: Erik Sundell <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few quick comments but in general this is looking great!
Co-authored-by: Erik Sundell <[email protected]>
Reposting this comment from @choldgraf as a reminder of TODO for myself:
|
I addressed this with another example and more explanation. I didn't want to overly complicate the issue, so I provided the building blocks of how you can achieve this with a more simple example and let the user decide what to do themselves. What do you think? |
Co-authored-by: Erik Sundell <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM, there are still some open discussions not yet resolved, but other than that I think this is excellent!
I'm very happy to have learned more about GitHub actions and MyST by reviewing this excellent documentation! Excellent work @hamelsmu! ❤️ 🎉
Co-authored-by: Erik Sundell <[email protected]>
Co-authored-by: Erik Sundell <[email protected]>
What is not resolved? Please let me know! |
Nothing any more, you quickly resolved them =) This LGTM! +1 for merge @choldgraf! |
Oh wait, maybe I messed up the title of this it is not supposed to be capitalized, it seems 🤦 my apologies EDIT: ☝️ Fixed it 🎉 LOL |
@consideRatio I'm going to start calling you the "human" linter 🎉 I got to get whatever IDE plugins you are using! Sharp eye for spacing |
Final fix so it doesn't get lost in the big thread: #203 (review) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM thanks @hamelsmu!!! ❤️
Co-authored-by: Tim Head <[email protected]>
Alright this LGTM as well - thanks all for all of the helpful comments and iteration! And congrats @hamelsmu on figuring out how Sphinx works! You are in rarefied air 😅 Thanks for this contribution! |
Summary of changes made:
myst_parser
todoc-requirements.txt
Thanks to @choldgraf for pair programming with me and getting me started.
cc: @betatim
Closes #202 (added by @consideRatio)