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

build: auto-create docs issues when PRs are merged #59546

Closed
jseldess opened this issue Jan 28, 2021 · 21 comments · Fixed by #66132
Closed

build: auto-create docs issues when PRs are merged #59546

jseldess opened this issue Jan 28, 2021 · 21 comments · Fixed by #66132
Assignees
Labels
A-build-system C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@jseldess
Copy link
Contributor

Today, the docs team uses this script to generate the markdown for public-facing release notes. Once a release notes file has been generated, the docs team uses this separate script to generate docs GH issues based on the items in the notes.

The second script saves us the effort of having to manually create docs issues based on engineering changes, which is great. But the fact that we use the script to generate docs issues en mass only when a release is ready to go out means that we don't create docs issues (and start documenting product changes) until a release goes out. Releases can get delayed quite a while, so this isn't optimal. For example, because the second alpha of 21.1 is going out more than a month after the first alpha, eng work that was done in mid-Dec won't get docs issues until late January.

Instead of relying on this script to generate docs issue en mass when a release is ready, it would be wonderful to automate the creation of docs issues more proactively, when a cockroach PR is merged. Here's my recommendation:

  1. When a cockroach PR is merged, check if any commits have a "Release note" text.
  2. If yes, generate a docs issue for each unique release note.

I'm not sure exactly how to implement this automation, though I imagine it would be part of CI or perhaps a Github action.

In terms of content, it would be fine to follow the same approach as we have in our current script. For each docs issue:

  • Title should match the title of the PR
  • Body should include a link to the PR as well as the release note text itself.
  • The C-release-note label should be applied.

The current script also assigns milestone to each issue based on script input. We could leave that out since it would be hard to automate.

Note: Once the Jira approach is figure out and implemented, we may change this approach once again. But in the meantime, this would help docs learn of changes sooner.

@jseldess jseldess added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Jan 28, 2021
@knz knz changed the title Auto-create docs issues when PRs are merged build: auto-create docs issues when PRs are merged Jan 28, 2021
@irfansharif
Copy link
Contributor

What about teaching the existing script that generates docs GH issues the ability to do it from a certain SHA, up until another SHA? That way we could periodically generate issues up until a certain point, and "catch up" the to the upcoming release, instead of doing it all en-masse.

@knz
Copy link
Contributor

knz commented Jan 28, 2021

That's valid -- but once we do this, we can also run that script as part of the CI merge run.

@irfansharif
Copy link
Contributor

irfansharif commented Jan 28, 2021

Agreed. My only gentle push back around doing any more complex work than necessary is that we've historically tied a lot of things in and out of our CI merge pipeline, and in a manner that's very opaque and unmaintainable. This is because these hooks typically live buried somewhere in TeamCity configs, and are nominally owned by our dev-inf team, but are primarily used and consumed by our docs team. From a workflow perspective, if I were on the docs team, I reckon it'd be more reliable to "pull in" the most recent batch of GH issues, and store the latest sampled SHA somewhere (likely in the repo itself), and just do that periodically. That's also closer to our existing workflow, but now amenable to doing it more frequently, which is what we want.

@knz
Copy link
Contributor

knz commented Jan 28, 2021

ok well at least as a first step we can extend the script to apply to a range of SHAs or perhaps a date range.

@jseldess
Copy link
Contributor Author

Thanks, @knz and @irfansharif. I defer to you all on the right pragmatic next step. In principle, though, ignoring the CI pains that Irfan mentions, I still believe that auto-creating docs issues at the time when a PR is merged is the best solution. Then the only manual effort for us is to triage new issues as they trickle in, rather than en mass (current approach) or in batches (Irfan's alternate approach).

Does it need to be part of the merge pipeline, or could it be a separate CI job triggered by a merge?

@knz
Copy link
Contributor

knz commented Jan 28, 2021

We also have a nightly script to send an e-mail to the eng team about all PRs that were merged that day. We can make it part of that.

@jseldess
Copy link
Contributor Author

An alternative I was discussing with @ianjevans would be to set this up on the docs side, as a github action, if it's possible to define an action to watch activity in another repo.

@kenliu kenliu self-assigned this Jan 30, 2021
@knz
Copy link
Contributor

knz commented Mar 15, 2021

@ianjevans I think your recent gh automation work is solving this right?

@ianjevans
Copy link
Contributor

Yes.
#61946

@knz
Copy link
Contributor

knz commented Mar 15, 2021

@ianjevans then it is useful to add "Fixes #59546" at the top of your PR description, so the two get linked to each other.

@jseldess
Copy link
Contributor Author

@knz, I think we still need to figure out if this is the exact automation solution in combination with our Jira migration plans. cc @elinorgarcia

@ianjevans
Copy link
Contributor

@jesse The Action can be modified later to file a JIRA issue in the appropriate repository. A GitHub Action consists of at least one job, and each job has a series of steps that are chained together.

So e.g. we could file both GitHub issues and JIRA issues if we wanted by adding another step, or replace the GitHub issue step with a JIRA step.

@jseldess
Copy link
Contributor Author

Just spoke with @elinorgarcia about this. Our migration to Jira will happen in a phased approach, pending adoption on the Engineering side. Specifically, our ability to automate the "context" for docs issues generated from release note texts depends on work on the Eng side still to come. In the meantime, we should definitely remove our manual work to generate issues from release notes via your Github action approach, and we do think we can use it as part of the future work to leverage Jira for docs. Let's work together to get your PR over the finish line.

@yzdocs
Copy link
Contributor

yzdocs commented May 5, 2021

Partial list of use cases to solve for:

Any combo of above can be present in one PR, of course. In all cases, the criteria is to only create docs issue if there is at least one release note that does not relate to bug fix or performance.

@yzdocs
Copy link
Contributor

yzdocs commented May 5, 2021

We also have a nightly script to send an e-mail to the eng team about all PRs that were merged that day. We can make it part of that.

@knz, could you please point me to this script? Thank you!

@knz
Copy link
Contributor

knz commented May 6, 2021

  • Release note duplicated across PR body and commit, where PR body and commit texts are not identical

We should be disciplined to ignore what's inside the PR "body" (it's really called "description" btw) -- it's very common for engineers to modify commits after the initial PR description has been written, to the point that the PR description becomes incorrect. In practice, the git commit message is nearly always a better source of truth.

  • Multiple release notes in PR body or one commit

PR body should be ignored, but yes it's common for single commits to contain multiple release notes

  • Release notes spanning multiple commits

Also think about commits without a release note.

Also think about release notes where the annotation contains a syntax mistake. The current script does extra work to correct common typos. This is an important quality-of-life benefit.

  • Multiline release note containing line breaks

Also think about release notes containing code examples, bullet lists and other markdown formatting. We want the formatting to be preserved.

@knz
Copy link
Contributor

knz commented May 6, 2021

could you please point me to this script? Thank you!

No, I cannot. I don't know where it is.

I do know however it's one of the microservices that the dev inf team is maintaining. Ask them where this "daily digest" generator lives.

@ianjevans
Copy link
Contributor

One alternative here is to modify release_notes.py to just output the RN text for a particular PR, since the functionality for parsing through the PR body and commit messages, including misspellings, is already there. It would require a new option for the script that only outputs the release note text for a particular PR.

The GitHub action would checkout the repo, set up Python (using e.g. actions/setup-python@v2, set up the refs in .git/config, pull origin, and run the script. The output would then be the body of the new docs issue.

@knz
Copy link
Contributor

knz commented May 6, 2021

It would require a new option for the script that only outputs the release note text for a particular PR.

This is already available, if you select the SHA from/until extremities using the PR branch name. (from memory, --from=master --until=refs/pulls/NNN, need to check)

@ianjevans
Copy link
Contributor

It would require a new option for the script that only outputs the release note text for a particular PR.

This is already available, if you select the SHA from/until extremities using the PR branch name. (from memory, --from=master --until=refs/pulls/NNN, need to check)

Interesting, that simplifies things.

But even if you set the various current --hide-x options it will still output the structure of the Markdown release notes file, right? So we'd need an option to skip all the structure, and only output the release note text(s).

@knz
Copy link
Contributor

knz commented May 6, 2021

it will still output the structure of the Markdown release notes file, right? So we'd need an option to skip all the structure, and only output the release note text(s).

good call, yes. that would be a good idea.

@craig craig bot closed this as completed in b60e941 Jun 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build-system C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
6 participants