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

Process docs updates #3855

Closed
tisonkun opened this issue May 3, 2024 · 5 comments · Fixed by #3937
Closed

Process docs updates #3855

tisonkun opened this issue May 3, 2024 · 5 comments · Fixed by #3937

Comments

@tisonkun
Copy link
Collaborator

tisonkun commented May 3, 2024

What type of enhancement is this?

Other

Background

When prototyping #3849 and developing the cyborg automation in #3852 and #3854, I found the following shortcomings in our current docs update process:

  1. It creates an issue on the docs repo when a label is created, but a label created by actions cannot further trigger the create issue action [1]. For example, there is no corresponding issue for feat: support evaluate expr in range query param #3823.
  2. It cannot identify if there are duplicate issues already.

[1] This is by design from GitHub Actions to prevent endless recursive event trigger.

Generally, automation add labels and users read the labels to take actions. Or Users add labels and trigger automation based on the label: but never mixed.

Proposal

I'd first hear from @sunng87 and @nicecui for how you currently handle cloud follow-ups and docs follow-ups and thus establish a clean but still solid way to handle these cases.

My current opinion is:

  1. Remove the cloud follow-up label. It is rare and we can simply create an issue in the cloud repo and refer to the related issues/prs.

  2. Replace the docs updates label with:

  • docs-todo
  • docs-done

We don't need a label for docs no-need. But we add a docs-todo label on - [ ] This PR does not require documentation updates.. Then @nicecui and @fengjiachun can ensure that all the necessary docs are written by moving all the docs-todo label to docs-done. Whether or not create an issue on the docs repo is an implementation details.

P.S. This is also how CockrochDB handles its docs process.

Looking forward to your thoughts. cc @killme2008

@tisonkun
Copy link
Collaborator Author

tisonkun commented May 3, 2024

To create issues conveniently, we can:

  1. Leverage the Reference in new issue button.
  2. Create a comment parse bot to accept /gtbot create-docs-issue (I'm not a big fan to such god-bot, but it's possible and sometimes helps).

@nicecui
Copy link
Collaborator

nicecui commented May 7, 2024

Thank you for your proposal.

Actually we learned from pulsar's pull request template about the process of tracking documentation updates.

I created a PR to let developers add doc labels manually, but people always forgot to add the label and the label checker always failed. Developers needed to check the failed information to understand that he needs to add a label named Doc update required or Doc not needed, that is cumbersome.

Then we created a new PR using the pull request template to notify developers to update document, and let the GitHub action to help us create documentation issues. The shortcoming is what you mentioned above: developers who updated the pull request description cannot further trigger the create issue action. Compared to the benefits, I think this is acceptable.

Then @nicecui and @fengjiachun can ensure that all the necessary docs are written by moving all the docs-todo label to docs-done. Whether or not create an issue on the docs repo is an implementation details.

hmmm...I think the document issues should be in the document repo, so the issues needs to be created. And I don't want to be responsible for everyone's doc label. The developer who responsible for features needs to ensure the documentation is updated. I also don't agree with the approach that people needs to maintain labels manually.

@tisonkun
Copy link
Collaborator Author

tisonkun commented May 7, 2024

@nicecui Thanks for sharing the background. Then I agree that we should follow the way to open issues on the docs repo.

Given that it's hard to de-duplicate issues created, and it's seldom duplicate issues created, I can update the action to create the issues when the content has been changed from - [x] This PR does not require documentation updates. to - [ ] This PR does not require documentation updates., while labels remain to help tagging, it won't trigger other actions. (Currently, we only create issues on "pull_request.created`, so if the property changes later, it won't create an issue. I suppose a few duplicate is better than missing.)

I'll ping you on the patch so that the proposal is more clear.

@nicecui
Copy link
Collaborator

nicecui commented May 7, 2024

Great! I can take responsibility for closing the duplicate issues in the documentation repo.

----------UPDATE----
Maybe - [x] This PR requires documentation updates. would be better than - [ ] This PR does not require documentation updates.

@tisonkun
Copy link
Collaborator Author

Maybe - [x] This PR requires documentation updates. would be better than - [ ] This PR does not require documentation updates.

Yes indeed. This is an interesting historical story:

  1. We first always assume docs is needed and require contributors to specify if docs is not needed.
  2. But later we found that mostly docs is not needed and contributors often forget to toggle the option and thus a lot of false positive issues created.
  3. We thus change the default content from - [ ] This PR does not require documentation updates. to - [x] This PR does not require documentation updates.. What a monkey patch XD.

I'll handle this with my patch, hopefully in this week.

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 a pull request may close this issue.

2 participants