Signed-off-by: Amita Sharma amsharma@redhat.com
Metadata Update from @jflory7: - Pull-request tagged with: needs changes, type - docs, type - internal organization
Metadata Update from @jflory7: - Request assigned
For AsciiDoc, the recommended practices suggest writing each sentence on its own line. This makes git diffs easier to review in future PRs that make changes to the docs content. It would be great to reformat the doc this way to help make things easier in the future.
This is minor, but since this is a URL anchor, it should be lowercase: [[process]]
[[process]]
I think lines 9-19 would look better as a description list. :smiley: Add a double-colon (i.e. ::) to the end of the bolded line. See the Description, multi-line section of the AsciiDoc syntax reference guide.
::
This creates this link (http://localhost:8080/diversity-inclusion/decision_process/Diversity#Team_Members), but it doesn't go anywhere. What page do you want to link to? If it exists on docs.fp.o, we can use a cross-document xref link.
xref
Hi @amsharma, thanks for improving the docs. :smile: This looks good to me and reflects the discussion we had previously.
I left a few comments in-line with suggestions. When you have a chance, please take a look at the feedback and then we should be ready to merge. In addition to the in-line comments, I have two additional comments:
-
_
nav.adoc
Using a hyphen affects the URL schema. Using hyphens over underscores is a subtle tip for better search engine optimization (SEO). And adding a link in nav.adoc will make sure it appears with the rest of our docs in the navigation sidebar.
Thanks again for working on this Amita. :tada: :tada: Let me know when you want me to take another look.
Hi @amsharma, did you have time to look at this PR again?
Hi @jflory7 whenever I come back to the issue I see this long list of comments and I go back to my other stuff 😓. Not that the comments are meaningless or not important but they somehow manage to keep me away to fix them..sorry took longer than expected but hopefully will do it this week. thanks.
rebased onto cd472bbd5db619c08103a51365a1f281a5736d3e
@jflory7 hey done, please check. Thanks for the review :)
I'm not sure what page we are linking here. Currently this is a 404 link when I test locally: http://localhost:8080/diversity-inclusion/decision-process/Diversity#Team_Members
Maybe it is better to not include a link here? I'm not sure where we publish a list of team members other than FAS.
@amsharma Thanks, your changes look great! :raised_hands: I caught one last thing while doing a follow-up review and left an in-line comment above.
I did a second read-over and I have more feedback on the process itself, but I will comment in #98 since my feedback doesn't block this specific pull request. Thanks again for working on this. :smile:
The only thing blocking this PR is a broken link. Since the PR is approaching a year old and there are no other blocking criteria, I am going to merge this and push up a new PR with a fix for the membership link.
Pull-Request has been merged by jflory7
Signed-off-by: Amita Sharma amsharma@redhat.com