Skip to content
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

feature and issue templates: change warning formatting #8102

Merged

Conversation

maxim-belkin
Copy link
Member

@maxim-belkin maxim-belkin commented Jul 27, 2020

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew tests with your changes locally?

This PR changes long titles to short titles + description. Spin-off from #8017

  • Feature template: link
  • Issue template: link

Copy link
Member

@Rylan12 Rylan12 left a comment

Choose a reason for hiding this comment

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

LGTM

One minor bikeshed (if that's the appropriate term 🤔) that shouldn't stop a merge

.github/ISSUE_TEMPLATE/bug.md Outdated Show resolved Hide resolved
@maxim-belkin maxim-belkin force-pushed the feature-template/shorter-titles branch from 17ee921 to 7d7b0f9 Compare July 27, 2020 15:01
Comment on lines 15 to 16
## Description
_What you were trying to do (and why)._
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure I see the advantage of having these on two lines instead of one?

@@ -4,19 +4,21 @@ about: "If you're sure it's reproducible and not just your machine: submit an is

---

# Please note we will close your issue without comment if you delete, do not read or do not fill out the issue checklist below and provide ALL the requested information. If you repeatedly fail to use the issue template, we will block you from ever submitting issues to Homebrew again.
**Please note we will close your issue without comment if you delete, do not read or do not fill out the issue checklist below and provide ALL the requested information. If you repeatedly fail to use the issue template, we will block you from ever submitting issues to Homebrew again.**
Copy link
Member

Choose a reason for hiding this comment

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

This seems fine 👍🏻

@MikeMcQuaid
Copy link
Member

This PR changes long titles to short titles + description.

Can you explain "why" rather than "what"? What problem for maintainers or contributors have you observed that will be fixed by this change?

@maxim-belkin
Copy link
Member Author

Can you explain "why" rather than "what"? What problem for maintainers or contributors have you observed that will be fixed by this change?

I can only speak about my motivation to make this suggestion. I believe that titles should be clear and succinct, such as "X" or "Do X". Such titles provide good anchor points in people's brains. Titles that include instructions (such as "include command output") or are complete sentences (such as "How the feature would be relevant ...") are less effective because they're convoluted with "secondary" words: the word "relevant" in the second example above appears in the sixth place and the whole point of that paragraph is "relevance" to 90% of Homebrew users. All the additional information and instructions can be provided elsewhere -- in a subtitle, comment, or a paragraph with futher instructions.

The benefit to maintainers is that they'll be able to say things like "In X section you say...". In general, short titles seem to be more visually appealing to me than the long ones.

@MikeMcQuaid
Copy link
Member

In general, short titles seem to be more visually appealing to me than the long ones.

On the flip side, splitting them from "long title" to "short title and description" makes cut content longer (vertically) which increases the feel of having more text to read and, the more text there is to read, the less likely people are to read any of it.

@maxim-belkin
Copy link
Member Author

maxim-belkin commented Aug 3, 2020

Comparison

  1. Current feature template
  2. Current feature template + H1->bold text change
  3. This PR
    image

Long titles + H1 headings actually make things appear to be longer. We can do the H1 -> H2 switch in feature template only but my point about the wordiness of titles remains -- I think shorter titles make it look more "professional".

@MikeMcQuaid
Copy link
Member

Given the lack of 👍🏻s from other maintainers here and the stalling: maybe close it if that's OK?

@maxim-belkin
Copy link
Member Author

Given the lack of 👍🏻s from other maintainers here and the stalling: maybe close it if that's OK?

@Homebrew/brew, could you guys please vote for (👍) or against (👎) this?

@MikeMcQuaid, if maintainers decide against this, I can open a separate PR for the change that you OK-ed.

@SeekingMeaning
Copy link
Contributor

On the flip side, splitting them from "long title" to "short title and description" makes cut content longer (vertically) which increases the feel of having more text to read and, the more text there is to read, the less likely people are to read any of it.

I agree with this

@reitermarkus
Copy link
Member

I think as long as the “long” titles fit into one line they are short enough.

@maxim-belkin
Copy link
Member Author

I think as long as the “long” titles fit into one line they are short enough.

That's what I want too: currently 3 out of 4 titles span 2 lines (and the first "Please note..." title shouldn't be really a title).

@reitermarkus
Copy link
Member

currently 3 out of 4 titles span 2 lines

Where? These all fit in 1 line for me when rendered.

@reitermarkus
Copy link
Member

Ok, not quite all. The only one which doesn't fit (apart from the “Please note …”) is the “How the feature would be relevant …” title.

@Rylan12
Copy link
Member

Rylan12 commented Aug 20, 2020

I, personally, like these changes. I feel more strongly about changing the "please note" text from H1 to bold. To me (as a maintainer), this text is just a distraction and there's no need to have it take up so much space and I find that it takes away from the rest of the content which is more important. The bold is a good compromise because it still clearly stands out from the rest of the text, and I'm not sure it's any less effective than H1. Remember that all text appears the same size and with the same emphasis in the Write tab when creating an issue. It's only when you switch to the Preview tab that you see the formatting. For me, this only happens at the end so I'm just as likely to read the text if it's a heading or bold.

I also like the other changes, but I don't feel as strongly about it and won't be upset if they aren't accepted. I think it looks much cleaner to have short and descriptive titles. This makes it much easier to glance at the section heading and immediately know what the contents are. I haven't memorized the order of the template, so I still need to read each heading, and it's much easier for me to read the one-word title and know immediately what that section is about than to have to read a whole sentence.

The way I see it, there are two sides to this. One is that we want to be clear about exactly what information we need and what we expect to contributors who are opening the issue. Is "The motivation for the feature" really any better than just having the word "Motivation"? Same with "Description". I'm not sure that having the entire sentence is any more descriptive (except maybe in the "Relevance" section). If anything, I would think that people are more likely to read smaller blocks of text than the entire sentence anyway. The bottom line is that people either read and follow it or they don't. I don't think that having full sentences makes people choose to follow the template when they otherwise wouldn't.

The second side is from the maintainer side. The maintainers should be able to easily read the issue using the template to get whatever information they need. There's no need for me to have to focus on a huge message warning about deleting the template. Same with the headings: I know what information I need, so to just have a single word heading makes it much easier. I can figure out which section I'm looking at much more quickly from the word "Relevance" than from the sentence "How the feature would be relevant to 90% of Homebrew users".

Anyway, sorry for the long comment. This is just my opinion but I have much less experience than many of the other maintainers so I may be in the minority.

@MikeMcQuaid
Copy link
Member

The bold is a good compromise because it still clearly stands out from the rest of the text, and I'm not sure it's any less effective than H1.

Fine with me.

Is "The motivation for the feature" really any better than just having the word "Motivation"? Same with "Description". I'm not sure that having the entire sentence is any more descriptive (except maybe in the "Relevance" section). If anything, I would think that people are more likely to read smaller blocks of text than the entire sentence anyway. The bottom line is that people either read and follow it or they don't. I don't think that having full sentences makes people choose to follow the template when they otherwise wouldn't.

Completely agree with this. I think adding more text that communicates essentially the same information across multiple lines makes it worse.

Comment on lines +7 to +9
# Bug report

**Please note we will close your issue without comment if you delete, do not read or do not fill out the issue checklist below and provide ALL the requested information. If you repeatedly fail to use the issue template, we will block you from ever submitting issues to Homebrew again.**
Copy link
Member

Choose a reason for hiding this comment

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

Can this PR be scoped down to just be these changes above (and same on feature suggestion)? If so: 👍🏻 to merge.

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Perfect, thanks @maxim-belkin and sorry for all the bike shedding 😬

@maxim-belkin maxim-belkin changed the title feature and issue templates: shorter titles feature and issue templates: change warning formatting Aug 27, 2020
@maxim-belkin maxim-belkin merged commit 5035e59 into Homebrew:master Aug 28, 2020
@maxim-belkin maxim-belkin deleted the feature-template/shorter-titles branch August 28, 2020 14:38
@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Dec 14, 2020
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Dec 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants