Skip to content

Conversation

@nienn
Copy link
Contributor

@nienn nienn commented Jan 18, 2022

Note: #27 is currently blocked on #23
Edit: #23 has been merged.

@nienn nienn requested a review from a team as a code owner January 18, 2022 17:22
@nienn nienn changed the base branch from main to nienn/home-page-template January 18, 2022 17:35

<!-- How it works -->
<section class="ui vertical segment very padded" id="how-it-works">
<div class="ui container center aligned">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't have anything here, it's probably best to remove this section and come back to it later. We want to be moving towards less mockup copy and content in the site.

Copy link
Contributor Author

@nienn nienn Jan 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Section removed.

Copy link
Contributor

@agjohnson agjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted a bunch of copy changes, but it was really helpful to separate the discussion on the layout of the page and come back to discuss the content separately.

<div class="item">
<i class="icon check circle green"></i>
<div class="content">
<div class="description">Issue Tracking</div>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I followed EA's lead and attached the Wikipedia style guide for content in this repo. For basically everything except h1's, we'd use sentence case. These bullet points should be cased down:

Suggested change
<div class="description">Issue Tracking</div>
<div class="description">Issue tracking</div>

https://en.wikipedia.org/wiki/Wikipedia:Manual_of_Style#Bulleted_and_numbered_lists

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These bullet list items seem to refer to something outside RTD. Perhaps we want to describe what RTD does here?

Copy link
Contributor Author

@nienn nienn Jan 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There where changes to this list introduced by PR #23, witch was being worked in parallel. I ported those changes here with commit 092f640. Please check to see if they feel like a good solution to this.

<div class="extra content">
<div class="ui labels">
<span class="ui label basic large"><i class="icon primary hourglass outline"></i> Always up to date</span>
<span class="ui label basic large"><i class="icon primary hourglass outline"></i> Automation</span>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Automation" sounds like a bit of a fragment. What is "automation" referring to here? It seems like there is probably a good deal of overlap between this and "Always up to date".

Copy link
Contributor Author

@nienn nienn Jan 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I'll remove "automation".

Still, its the same problem as in the technical writer features section. We need to convey how RTD can add value to technical teams and I have no better suggestions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Let's come back to this one with more copy review

Copy link
Contributor Author

@nienn nienn Jan 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened issue #46 to help keeping track of this.

@nienn nienn mentioned this pull request Jan 20, 2022
@nienn nienn force-pushed the nienn/home-page-template branch from 0e029e5 to 6040ecf Compare January 25, 2022 15:50
@nienn nienn mentioned this pull request Jan 25, 2022
@nienn
Copy link
Contributor Author

nienn commented Jan 25, 2022

Noted a bunch of copy changes, but it was really helpful to separate the discussion on the layout of the page and come back to discuss the content separately.

I absolutely agree! It's really hard to review copy in the same PRs that we review code and project scaffolding and also more prone to create conflicts with other PRs or loose important thought processes.

Is there anything I can do to promote separate discussions?

I first thought of these latest PR batch as simply porting the mockup/s code to the template/s logic. We ended up revising a lot more at once and, again, seem to have fallen into the trap of the big PR. This PR is now introducing a lot of new changes but it's still far from reflecting a final polished version of the page, because there are still a lot of elements missing: like links, images, javascript, style adjustments, template logic, etc.

I propose we try to close these PRs as a second mockup and come back to them with separate PRs, each focused on one single objective, to polishing each technical, conceptual, graphic and scaffolding aspects separately.

@agjohnson
Copy link
Contributor

I propose we try to close these PRs as a second mockup and come back to them with separate PRs, each focused on one single objective, to polishing each technical, conceptual, graphic and scaffolding aspects separately.

@nienn That's fair! Would it help to maybe open an issue for each page that have ongoing copy work then? We have been merging development level display changes lately, so what is merged isn't yet complete already. So, it's fine to be merging more development level copy as well then, but I do want to keep up momentum on completing the pages.

I would hold off on tweaking any graphics until we have copy though, there are a few places where graphics are meant to match copy, and the copy could change.

@nienn
Copy link
Contributor Author

nienn commented Jan 26, 2022

Would it help to maybe open an issue for each page that have ongoing copy work then?

Yes! This absolutely helps. We might become a bit overwhelmed with these issues soon thought. Maybe we can start adding issues to milestones and/or add appropriate labels to keep track of the ones that are required before any release. Also maybe start adding labels to distinguish between development, new feature and bug issues.

I would hold off on tweaking any graphics until we have copy though, there are a few places where graphics are meant to match copy, and the copy could change.

Yes, I agree. I would only add that its important to know where there will be a graphic and the general shape/concept of that graphic, as that will alter the copy too. Both should live together.

@agjohnson agjohnson changed the base branch from nienn/home-page-template to main January 26, 2022 18:18
@agjohnson
Copy link
Contributor

Maybe we can start adding issues to milestones

Yeah, I was going to ask if milestones would be helpful. We are accumulating issues and prioritization would help. We can also move things to the roadmap view in Trello if you'd prefer. I'm open to either.

Feel free to apply any of the labels that you think make sense. We already use some of the labels you mention, if you haven't seen it yet, our issue triaging guidelines are here: https://dev.readthedocs.io/en/latest/issue-labels.html

@agjohnson
Copy link
Contributor

I updated the base to main but this branch could use a rebase or merge of main, to isolate the pr changes here

@nienn
Copy link
Contributor Author

nienn commented Jan 26, 2022

I'm going to commit these two suggestions - #28 (comment) and #28 (comment) - and rebase.

@nienn nienn changed the base branch from main to nienn/home-page-template January 27, 2022 17:22
@agjohnson agjohnson changed the base branch from nienn/home-page-template to main January 27, 2022 19:39
Copy link
Contributor

@agjohnson agjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also looks ready to merge! If there are improvements to the copy etc, we'll address these with some iteration.

@agjohnson agjohnson merged commit d7a6b50 into main Jan 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants