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

Discuss Documentation Policy #12940

Closed
Tracked by #12916
jzheaux opened this issue Mar 28, 2023 · 6 comments
Closed
Tracked by #12916

Discuss Documentation Policy #12940

jzheaux opened this issue Mar 28, 2023 · 6 comments
Assignees
Labels
for: team-attention This ticket should be discussed as a team before proceeding in: docs An issue in Documentation or samples type: enhancement A general enhancement

Comments

@jzheaux
Copy link
Contributor

jzheaux commented Mar 28, 2023

We should consider how we want to ensure docs are up to date going forward.

One way is to add a status: needs-documentation label. It would be added when the issue is triaged. It would be removed once documentation is added or when a follow-up documentation ticket is created. In the latter case, the label is moved to the follow-up ticket. We should also document how we use this label. :)

Discussion around the proposal as well as alternative ideas are welcome below.

@jzheaux jzheaux added in: docs An issue in Documentation or samples type: enhancement A general enhancement labels Mar 28, 2023
@jzheaux jzheaux self-assigned this Mar 28, 2023
@jzheaux jzheaux added the for: team-attention This ticket should be discussed as a team before proceeding label Mar 28, 2023
@jzheaux jzheaux changed the title Add Documentation Guidelines Consider a Documentation Label Mar 28, 2023
@rwinch
Copy link
Member

rwinch commented Mar 28, 2023

Is there a reason we need a label? I'd prefer if we just require it before pushing code.

@jzheaux
Copy link
Contributor Author

jzheaux commented Mar 28, 2023

Good question. I think it depends, as you alluded to, on how flexible we want to be with the requirement. If documentation work can be deferred, then a label is needed to ensure the deferred work is tracked. I'm interested to hear everyone's preferences here.

I also like the idea of treating them similarly to unit tests, which I think is closer to what you are proposing. Consider this other policy that @marcusdacoregio and I came up with:

  • All contributions should update the affected documentation
    • All New Features must include documentation
    • All bug fixes must update any now-erroneous documentation

This one is similar to our unit test policy, which is that all new features must include tests and all bug fixes must have a test that verifies the now-corrected behavior.

Note that with any policy, sometimes documentation is not needed since some fixes are about bringing the code in alignment with existing docs.

@jzheaux jzheaux changed the title Consider a Documentation Label Discuss Documentation Policy Mar 28, 2023
@marcusdacoregio
Copy link
Contributor

I think that the needs-documentation label is more helpful for external contributors than it is for the maintainers. In my opinion, it is less productive that someone opens a PR for example and then we tell them that the documentation should be also updated. If we label was added beforehand, the contributor will be remembered, just by looking at the issue, that they need to consider it.

Although this does not solves it 100%, it is more transparent with external contributors.

@jzheaux
Copy link
Contributor Author

jzheaux commented Mar 29, 2023

it is less productive that someone opens a PR for example and then we tell them that the documentation should be also updated

Agreed, though some of this may be about education. We don't have a needs-tests label, for example, and it works to give the occasional reminder to folks to add unit tests.

That said, what I'm picturing is actually something different from that. IMO, a maintainer would be the one to push a documentation polish commit to most contributor PRs before merging. I think requiring docs from external contributors would be too discouraging, especially for folks who don't feel strong enough in their English to do so.

Of course, if a PR has a needs-documentation label, the contributor is more than welcome to add documentation. What I personally like about the label is the reminder it would give me that no one (though probably specifically the maintainer) has added the needed polish commit yet.

@jzheaux jzheaux moved this to In Progress in Spring Security Team Apr 4, 2023
@jzheaux
Copy link
Contributor Author

jzheaux commented May 31, 2023

We discussed this a bit more in our retro and here are some additional thoughts that came out of that:

  • The ideal is that documentation is added before the PR is merged or before the issue is closed. In those cases, no extra label or issue is needed.

In unideal scenarios, documentation is still needed. The rest of this post is about what to do in this case.

  • Some team members like creating a follow-up documentation issue, while others like reopening the original issue.
  • In both cases, I advocate for a status: needs-documentation label as a quick, low-ceremony way to tag a closed issue/PR as a reminder that either 1. no follow-up documentation ticket has been created yet or 2. this issue is due to be reopened at a later time.

Speaking for myself, I prefer to commit the documentation to the original issue (not create a separate issue). I think this is easier than creating a separate issue, there is less clutter, it makes it easier to read our release notes, and it's more in the spirit of the ideal. Adding a label to mark those issues makes it easy to search for them and ensure that all documentation is added before the next release.

@jzheaux
Copy link
Contributor Author

jzheaux commented Apr 17, 2024

Given the above, here's the policy I suggest:

As part of resolving any ticket, please include any necessary documentation changes. If you cannot do this--say due to scheduling constraints--please mark the ticket with status: needs-documentation and then, once you have time: Reopen, add the needed documentation commit, and remove the label.

@jzheaux jzheaux moved this from In Progress to Done in Spring Security Team Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: team-attention This ticket should be discussed as a team before proceeding in: docs An issue in Documentation or samples type: enhancement A general enhancement
Projects
Status: Done
Development

No branches or pull requests

3 participants