-
Notifications
You must be signed in to change notification settings - Fork 3
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
Documentation #50
Documentation #50
Conversation
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.
Looks good for the most part!
docs/index.rst
Outdated
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.
Just wondering, but how are you finding writing docs in RST?
Wagtail's own docs have been migrated from RST to Markdown using MyST to be more accessible to contributors.
In this package's case, I don't imagine there's going to be lots of pages, so I think using RST is fine. I'm just highlighting the option in case you'd prefer Markdown to RST.
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.
I might have been exposed to RST before Markdown, and also assumed Sphinx only supported RST (I think this was the case for a long time). Had I known I could have used Markdown, I would have probably started down that route, I don't think there's anything in these docs that couldn't have been done with Markdown.
|
||
|
||
class StreamField(WagtailStreamfield): | ||
class StreamField(WagtailStreamField): |
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.
Curious to know what this override really is for, I don't understand why a demo project needs something like this 🤔
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.
Oh, it's just to avoid migration noise from changes in the StreamField blocks definition. Originally I expected to be adding several block types in sequence to demo their features.
@@ -17,6 +17,7 @@ export WAGTAIL_NEWSLETTER_MAILCHIMP_API_KEY=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx-us1 | |||
export WAGTAIL_NEWSLETTER_FROM_NAME="My Newsletter" | |||
export [email protected] | |||
|
|||
python -m pip install --editable='.[testing,mailchimp,mrml]' |
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.
Just realised we do have a CONTRIBUTING.md
... But OK, I think this is a small worthwhile improvement 🙂
Fixes #26
Fixes #52
Preview on Read the Docs: https://wagtail-newsletter--50.org.readthedocs.build/en/50/