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

Expand "What's New and News Entries" to include Sphinx #525

Merged
merged 13 commits into from
Aug 23, 2019

Conversation

aeros
Copy link
Contributor

@aeros aeros commented Aug 16, 2019

Based on the feedback from the ML thread in Python-Dev and Discuss topic it seems appropriate to update the devguide accordingly.

I also adjusted the number of spaces in the indentation from the existing example from 2 to 3 in order to meet reST standards.

@aeros aeros changed the title Expand news entries sphinx Expand news entries to include Sphinx Aug 16, 2019
@aeros aeros changed the title Expand news entries to include Sphinx Expand "What's New and News Entries" to include Sphinx Aug 16, 2019
@methane
Copy link
Member

methane commented Aug 17, 2019

To make clear the role usage is not strongly recommended, I want to add this paragraph:

Useful NEWS entries are copied to "What's New" and edited for wide readers anyway.
Most NEWS entries are just like commit logs.  No need to care much about using roles.

Suggestion from Inada Naoki.
@aeros
Copy link
Contributor Author

aeros commented Aug 17, 2019

@methane

To make clear the role usage is not strongly recommended, I want to add this paragraph:

Useful NEWS entries are copied to "What's New" and edited for wide readers anyway.
Most NEWS entries are just like commit logs.  No need to care much about using roles.

Thanks for the feedback.

While I can definitely see that the current section might come across a bit too strongly with recommending the usage of the roles, using "No need to care much about using roles" might have the exact opposite issue. If such wording such as "anyway" and "no need to care" is used, then it will be highly unlikely that the feature will utilized at all, ultimately defeating the purpose of adding it in the first place. I think that we may need to find a middle ground of sorts, where the section promotes appropriate usage of the roles without coming across too strongly.

Although the "What's New" section is far more visible to users, it is created using the contents of Misc/NEWS.d/, which has files composed from the blurbs for each version. As far as I'm aware, the majority of contributors have no means of directly making modifications to "What's New". That would leave the responsibility of adding/reviewing the Sphinx roles to the release managers, who be rather unlikely to have time to do so.

As a result, I think the easiest way to go about adding the roles is by either:

A) Including them in the news entries for PRs
B) Submitting a PR for each file in Misc/NEWS.d/ to include to them

For the majority of cases, I think it's far easier to include them in the PR itself (especially if a someone familiar with using the roles directly recommends them as a suggestion to the news entry or if the PR author has experience with documentation). However, it is inevitable that many PR authors will not be overly familiar with the syntax and will have their PR merged before someone adds it as a suggestion. In those cases, they can be added retroactively through their respective file in Misc/NEWS.d/.

However, I think it would be quite impractical if PR authors rejected news entry suggestions to add the roles because "they can just be added later". In my experience thus far from working in various projects, "later" all too often ends up becoming "never". Additionally, adding all of the Sphinx roles later would result in adding a significant amount of additional time to review the PR adding them, reducing it's chances of being merged.

Based on my experiences thus far from both submitting PRs and reviewing them, there far more PRs submitted than there are people to review them. A rather large number of those end up becoming buried and not reviewed by committers until several months later (which happens particularly often for documentation-related PRs). If we can mostly avoid adding to that problem by simply including the markup in the news entries, I think that we should aim to do so.

@methane
Copy link
Member

methane commented Aug 17, 2019

Although the "What's New" section is far more visible to users, it is created using the contents of Misc/NEWS.d/, which has files composed from the blurbs for each version.

It's not true. NEWS is created NEWS entries by blurb. But "What's New" is edited separately.

The author of "What's New" sometime just copy NEWS entry. Sometime rewrite the entry completely because NEWS and What's New has different readers. And committers sometime write both of "What's New" entry and "NEWS" entry sometime.

@aeros
Copy link
Contributor Author

aeros commented Aug 17, 2019

@methane

It's not true. NEWS is created NEWS entries by blurb. But "What's New" is edited separately.

The author of "What's New" sometime just copy NEWS entry. Sometime rewrite the entry
completely because NEWS and What's New has different readers. And committers sometime
write both of "What's New" entry and "NEWS" entry sometime.

Ah, thanks for the correction. I was under the impression that "What's New" was the result of selectively copied NEWS entries, rather than being separately written. Is it possible for non-committers to directly assist with "What's New" similarly to how they can with Misc/NEWS?

Regardless of the NEWS -> "What's New" transition process though, my primary concerns still remain:

If such wording such as "anyway" and "no need to care" is used, then it will be highly unlikely that the feature will utilized at all, ultimately defeating the purpose of adding it in the first place.

However, I think it would be quite impractical if PR authors rejected news entry suggestions to add the roles because "they can just be added later". In my experience thus far from working in various projects, "later" all too often ends up becoming "never".

Based on my experiences thus far from both submitting PRs and reviewing them, there far more PRs submitted than there are people to review them. A rather large number of those end up becoming buried and not reviewed by committers until several months later ... If we can mostly avoid adding to that problem by simply including the markup in the news entries, I think that we should aim to do so.

@methane
Copy link
Member

methane commented Aug 17, 2019

Ah, thanks for the correction. I was under the impression that "What's New" was the result of selectively copied NEWS entries, rather than being separately written.

For example, this is my PR which added both of a NEWS entry and a What's New entry.
https://github.com/python/cpython/pull/12884/files?file-filters%5B%5D=.rst

Is it possible for non-committers to directly assist with "What's New" similarly to how they can with Misc/NEWS?

Of course. It can be done by normal PR workflow.

If such wording such as "anyway" and "no need to care" is used, then it will be highly unlikely that the feature will utilized at all, ultimately defeating the purpose of adding it in the first place.

But current document looks like it is recommended.

This document should teach people the roles can be used, but should not recommend to use it.
So it's important that its usefulness is very limited compared to general documentation improvement.

However, I think it would be quite impractical if PR authors rejected news entry suggestions to add the roles because "they can just be added later". In my experience thus far from working in various projects, "later" all too often ends up becoming "never".

I'm OK about it. Not using roles is not a big usability issue.

Based on my experiences thus far from both submitting PRs and reviewing them, there far more PRs submitted than there are people to review them. A rather large number of those end up becoming buried and not reviewed by committers until several months later ... If we can mostly avoid adding to that problem by simply including the markup in the news entries, I think that we should aim to do so.

If committer should check the link is correct before merging the PR, it can increase review cost too.

@methane
Copy link
Member

methane commented Aug 17, 2019

I think this PR should be merged after netlify preview is activated.
Run "make html" locally is huge pain for reviewer. I check out the PR only when it's really needed. I review the code only on Github in 80% PRs.

@terryjreedy
Copy link
Member

I believe most coredevs (including me) are not, or at least have not been "What's New" conscious. Traditionally, many items have added later by an editor reviewing the news items. (For all I know, trivial items might even be deleted.) I do "What's New" for IDLE in separate PRs because of its special complications.

Editors also add headers and review existing items for placement and style consistency. There may not be an issue for editor PRs, and they might be created and merged after CI, with little time for public review. The latter is true of my "What's New"-only PRs, and I do edit after copying blurbs.

@aeros
Copy link
Contributor Author

aeros commented Aug 17, 2019

For example, this is my PR which added both of a NEWS entry and a What's New entry.
https://github.com/python/cpython/pull/12884/files?file-filters%5B%5D=.rst

I believe most coredevs (including me) are not, or at least have not been "What's New" conscious. Traditionally, many items have added later by an editor reviewing the news items.

Interesting, I have not regularly seen the majority of PR authors or core devs directly adding to entries "What's New". I think it's a good idea for larger changes, but it's definitely not standard practice for most. As a result, I think we should recommend the usage of the roles for the entries in Misc/NEWS when it's not overly inconvenient.

But current document looks like it is recommended.

This document should teach people the roles can be used, but should not recommend to
use it.
So it's important that its usefulness is very limited compared to general documentation
improvement.

I'm OK about it. Not using roles is not a big usability issue.

I think that we should encourage the usage of the roles, but without strictly requiring them. As stated in my post over on discuss in more detail, I think that the inline links can make it significantly easier for readers to find relevant documentation. If it's not a significant inconvenience to the author, they should be included when possible.

Is there a way you would recommend changing it further to make it sound less like the roles are required without discouraging the usage of them? That was my main concern about the section you recommended adding at the end.

Run "make html" locally is huge pain for reviewer. I check out the PR only when it's really needed. I review the code only on Github in 80% PRs.

I think most people do this as well. I put this in there based on the recommendation from Ned in the ML, but he may have been specifically referring to editing of the Misc/NEWS files that contain a larger number of news entries rather than doing it for individual PRs. I can remove that part later tonight. Checking to make sure there's a corresponding entry in the docs should be adequate for single entries.

@methane
Copy link
Member

methane commented Aug 18, 2019

As a result, I think we should recommend the usage of the roles for the entries in Misc/NEWS when it's not overly inconvenient.

It totally doesn't make sense.

  • What's New is not a just copy of NEWS. For example bpo-12345 is used in NEWS but :issue:`12345` is used in What's New. Generally speaking, "What's New" should be more descriptive than NEWS.

  • The "What's New" documents only important changes between 3.X and 3.X+1. On the other hand, NEWS (changelog) records all changes like between 3.X.Y and 3.X.Y+1rc1. Bug fixes are recorded in changelog, but very unlikely documented in "What's New". Recommending roles in 10000 NEWS entry to help using roles in 100 What's New entry doesn't make sense at all. It is too bad performance / cost ratio. You can just help to edit "What's New" instead of NEWS.

So "What's New" must not be a reason for using roles in NEWS.
Only reason to use roles in a NEWS entry is just readability of the NEWS entry, not "What's New".

I think that we should encourage the usage of the roles, but without strictly requiring them. As stated in my post over on discuss in more detail,

I still strongly oppose to this.

There are no consensus about recommend/encourage the usage of the roles in the discussion.

The only consensus made in the discussion is just documenting about roles can be use in NEWS in the devguide. So it shouldn't be recommended/encouraged at this point.

I think that the inline links can make it significantly easier for readers to find relevant documentation. If it's not a significant inconvenience to the author, they should be included when possible.

As I said in the discussion, the number of readers of each entry is very small than a regular document. Many OSS projects still use plaintext for their changelog, or even don't have changelog document.

I don't think effort for using/maintaining roles in NEWS correctly worth enough, especially when the change is just a small bugfix or performance improvement. The most important context for the changelog is the issue, and it is linked in all NEWS entries already.

@aeros
Copy link
Contributor Author

aeros commented Aug 18, 2019

@methane

Recommending roles in 10000 NEWS entry to help using roles in 100 What's New entry doesn't make sense at all

As I said in the discussion, the number of readers of each entry is very small than a regular document

I can see what you mean, but I still think there's some benefit to having them in the NEWS entries. Even if there's less benefit for NEWS, it is still easier to click a link for more information than it is to dig through an entire bpo issue's discussion or find the corresponding section in the documentation.

You can just help to edit "What's New" instead of NEWS.

There are no consensus about recommend/encourage the usage of the roles in the discussion.

The main reason that made me think Misc/NEWS entries should be edited directly and that the roles should be encouraged was partly because of how @ned-deily responded in the ML thread:

Also, a related question: would it be helpful for contributors to look through news entries for the latest stable and beta releases, and add inline links where they could be useful for readers?

We haven't done a lot of that in the past but I don't see a reason not to, especially since it is easy on most systems to generate the changelog by building the html docs in the source tree

One important note: to avoid confusion, only edit the blurb NEWS rst files in the branch for that release, i.e. edit Misc/NEWS.d/3.8.0b1.rst in the 3.8 branch, not in the master branch.

and how @brettcannon responded in the Discuss topic:

In general, should Misc/NEWS entries in PRs aim to provide links to documentation related to the changes made using inline reST markup?

If the build of Misc/NEWS supports it then doing the appropriate markup makes sense to me.

I might be misunderstanding what they meant though, so maybe they can provide further context.

The only consensus made in the discussion is just documenting about roles can be use in NEWS in the devguide. So it shouldn't be recommended/encouraged at this point.

Apologies, perhaps I have been a bit overzealous then in recommending the roles. I'll go over the section and try to remove any language that can be interpreted as a recommendation. I just want to avoid discouraging it, as I think that will result in the feature not being used anywhere.

I still strongly oppose to this.

I will attempt to revise the section further after the others have responded, and let you know once I've updated it. Thank you for your feedback and I apologize if I have been overly passionate about the usage of the roles. This was my first proposal/idea in the ML that was accepted, so I may have gotten a bit too overeager about trying to get everyone to use them more.

Thank you for continuing to provide detailed responses.

@methane
Copy link
Member

methane commented Aug 18, 2019

I can see what you mean, but I still think there's some benefit to having them in the NEWS entries.

You don't see what I mean. I never declined there is some benefit in the NEWS entries. I just said the benefit is smaller than regular document.

The point is "benefit / effort" ratio. Is it higher than other tasks? Note that even you will contribute to edit NEWS entries, committers should pay effort to review and maintain it.

That's why I don't want to encourage/recommend it. Let's start small, instead of encouraging it to everyone.

@terryjreedy
Copy link
Member

I have not followed the substance of this issue, but I agree with 'start small'.

Adjust make HTML section to specify when it's needed and adjust wording to remove explicit recommendation to use the Sphinx roles by changing "should" -> "can".
@aeros
Copy link
Contributor Author

aeros commented Aug 18, 2019

@methane

That's why I don't want to encourage/recommend it. Let's start small, instead of encouraging it to everyone.

In order to remove the explicit recommendations to use the Sphinx roles, I changed all instances of "should" -> "can".

Run "make html" locally is huge pain for reviewer. I check out the PR only when it's really needed. I review the code only on Github in 80% PRs.

I changed the "make HTML" section to not recommend doing so for individual PR news entries. The "make HTML" process should only be needed when editing multiple entries at once in "What's New" or Misc/News.d/ files, based on Ned's response in the ML thread.

Additionally, I added a section which mentions that "What's News" benefits the most from the Sphinx roles, since it has the largest volume of readers.

Focus exclusively on news entries, adjust make HTML section, and mention that inline code blocks can be used as a substitute.
@aeros
Copy link
Contributor Author

aeros commented Aug 21, 2019

@methane I finished making the recommended changes. Let me know if you have any other suggestions. (:

@methane
Copy link
Member

methane commented Aug 22, 2019

I still feel you're talking too much thing to recommending roles.
I want to keep this section simple, necessary and sufficient for new contributors.

I created the pull request to simplify this section.
aeros#1

@aeros
Copy link
Contributor Author

aeros commented Aug 22, 2019

I created the pull request to simplify this section.

Okay, sounds good. I'll look over it to see if I have any recommendations/feedback and then merge it into this PR. Thanks.

@aeros aeros mentioned this pull request Aug 22, 2019
Significantly shortens the added section to be more succinct.
@aeros
Copy link
Contributor Author

aeros commented Aug 23, 2019

@methane I merged your PR into this one. Are we good to move forward with this?

@methane methane merged commit a19887a into python:master Aug 23, 2019
@aeros
Copy link
Contributor Author

aeros commented Aug 23, 2019

Thanks again for the feedback!

AA-Turner pushed a commit to AA-Turner/devguide that referenced this pull request Jun 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants