-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Cookbook entry for Display component #173
Conversation
Apperently I didn't get the links correct 10:46:31 AM: /opt/build/repo/cookbook/display.rst:14: WARNING: undefined label: /components/display/ssd1306_i2c (if the link has no caption the label must precede a section header) |
Noticed the warning about tabs vs spaces, have fixed them :-) |
Fixed some syntax errors in reStructuredText which is totally new for me.
Fixed some more issues
Hi, Need help Otto :-) Get these warnings (possible some more but I think I fixed them) /opt/build/repo/cookbook/display.rst:29: WARNING: Content block expected for the "warning" directive; none found. Don't know what is wrong /opt/build/repo/cookbook/display.rst:119: WARNING: unknown document: https://esphome.io/api/display_8h.html I understand it is wrong syntax but didn't know how to properly refer to another repo File cookbook/display.rst contains tab character on line 47:0. Please convert tabs to spaces. I have removed all tabs and replaced with 4 spaces but still throwing an error. |
I have another image to use in the table of content. Once this page works fine I'll update the toc page too. |
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.
Thanks! Some comments:
- The name "display" for the cookbook entry is a bit too generic. I'd prefer something more specific like "Time & Temperature OLED Display"
- The image needs to be resized a bit - even if it's not on the main page 2MB is quite big (and slows down every people downloading the docs to edit them locally)
- In the opening paragraph, it would be useful to list what this config will do, it doesn't have to be long just a short list so that people don't need to read through the entire page (image is also good like it is already, but a bit of text too won't hurt)
Great. Again, I'm new to github so I think I just reverted the updates you made. I'll fix that (as well as your review comments - including the size of the second file I just uploaded) |
- Updated the title and renamed the file - Resized the images
- highlighted that 'model' also could be different - small cosmetic changes
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.
👍 The guide itself looks good now. The two images could be renamed though to match the new cookbook entry name.
And you will have to add it to the index.rst
file so that it is listed.
Done the changes discussed. Possibly the title in the index (which matches the page title) is too long - I don't know how it will be rendered - shorten it if you like! Thanks for all support with my first pull request! |
Looks like the title become two liner but looked okay in my view. The image was not displayed correctly though - is that due to the "rendering preview engine" or something that I should fix? |
yeay, finally! Everything looks okay now as far as I can see! |
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.
🎉
Not a problem in this case. |
I created a cookbook entry how to use the display component.
(First github pull request and first time using reStructuredText so if it isn't up to the mark, please let me know what to fix and I'll do that and send another pull request)