Repository navigation
Contribute page review structure & checklist - #1117
Conversation
- Expands on the process for requesting reviews and proposes a 3-step structure - Add a common issues checklist for general and smaller things to look out for before requesting reviews
✅ Deploy Preview for bitcoin-design-site ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Did a first pass review, and overall it looks pretty good. Really appreciate how you've incorporated much of what I expressed during the silent payments PR review process. I've dropped a few suggestions and comments.
- One recommendation that I have may be added somewhere if it makes sense: keep Structural (or even Refinement stages) off the PR/github especially for bigger items, and open the PR only when you're half way or further.
During the initial stages, the content is likely to change/move substantially and line-level feedback might not be needed or provided. It also keeps the content more accessible to folks who don't prefer Github.
-
I'd also like to see more/obvious cross-links to related pages
-
I was wondering if it might make sense to have a sample page .md file that has templated content including lots of different elements such as those mentioned on Content, Image and Formatting guideline pages.
Currently, one starts by searching for a good template/sample page to clone (at least I do dunno about others) but then still have to go hunting for code/template for other type of elements.... and a template/sample page might help folks get started...
| - Lowercase "bitcoin", "lightning network" and "ecash" | ||
| - Make sure to explain less-widely known technical terms before expecting users to understand their meaning and properties | ||
| - If a new page is a sub-section, make sure to include a reference in the section landing page ([example](https://bitcoin.design/guide/how-it-works/)) | ||
| - A simpler writing style allows your content to be more accessible to more readers (especially ones for whom english is not their native language) |
There was a problem hiding this comment.
Not sure this belongs in the checklist per se. IIRC this is also explained in the Content guidelines section.
There was a problem hiding this comment.
I think that's fine. This checklist is a bit of a summary of things that are explained in more detail elsewhere, so some repetition is OK.
| - Ensure "width" and "height" attributes in picture and image includes match the actual image dimensions | ||
| - For smallest file size, use the JPG file format for textured and photographic images, and PNG for images with mostly flat color | ||
| - Compress your images via tool like [ImageOptim](http://imageoptim.com) (can save ~70% of bandwidth) | ||
| - Delete unused images from your PR |
There was a problem hiding this comment.
This might be hard to detect if a mistake is made.
There was a problem hiding this comment.
Yep, but it has to be done.
| - Ensure image paths are correct | ||
| - Ensure "width" and "height" attributes in picture and image includes match the actual image dimensions | ||
| - For smallest file size, use the JPG file format for textured and photographic images, and PNG for images with mostly flat color | ||
| - Compress your images via tool like [ImageOptim](http://imageoptim.com) (can save ~70% of bandwidth) |
There was a problem hiding this comment.
This should be done just before merging or after all images are finalised...otherwise user has to remember to do it every time they change an image... not sure how this can be communicated.
There was a problem hiding this comment.
If you use a tool like ImageOptim, it will not compress images twice. Just drag all your pics in, let it do its thing, and you're good to go.
|
Sorry the following is feedback for the page/topic in general and not limited to this PR I also wonder how much GitHub must be a part of 'proposing a change'. Much of the content here is likely to be applicable whether or not somebody opens a PR. It can also be a steep learning curve for non-technical people, and I'd figure we want to make this process as accessible as possible. If the above makes sense, perhaps the checklist might appear somewhere else maybe higher up on the page but possibly more changes needed then... |
Thanks to Yashraj for finding my various spelling mistakes and suggesting good tweaks. Co-authored-by: Max <114083781+yashrajd@users.noreply.github.com>
Tweaking the rendering of relative links, so the code is not pre-processed. The syntax shown on the site is now correct. Also adding an explainer for the reason why this syntax is necessary.
mouxdesign
left a comment
mouxdesign
left a comment
There was a problem hiding this comment.
Made some minor suggestions other than that LGTG
Co-authored-by: Mogashni <90313394+mouxdesign@users.noreply.github.com>
I complained a bit yesterday about some common issues I find across PR reviews, and Yashraj recommended creating a checklist. So here it is.
This PR expands on the process for requesting reviews and proposes a 3-step structure. It also adds a common issues checklist for general and smaller things to look out for before requesting reviews.
I am keeping this as a draft as I'd like to share it for now, but also do another iteration specifically looking out for cross-linking and referencing across other pages in the contribute section. Also curious to hear if others have items they'd like to put on the checklist.
🍓Check the preview🫐