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

[Issue #1820] Syncs ADRs with wiki #1826

Merged
merged 12 commits into from May 2, 2024
Merged

Conversation

widal001
Copy link
Collaborator

@widal001 widal001 commented Apr 24, 2024

Summary

Moves documentation/decisions/ to documentation/wiki/decisions/ so that future ADRs will show up in both GitBook and in the repository.

Fixes #1820

Time to review: 5 mins

Changes proposed

What was added, updated, or removed in this PR.

  • Moves documentation/decisions/ to documentation/wiki/decisions/
  • Adds a adr/README.md file so that wiki users can browse all of the ADRs
  • Renames infra/index.md to infra/README.md to do the same for Infra ADRs
  • Updates SUMMARY.md so that the newly synced files will appear in the wiki UI
  • Adds ci-wiki-links.yml to check that all files in documentation/wiki/ are in SUMMARY.md

Context for reviewers

Testing instructions, background context, more in-depth details of the implementation, and anything else you'd like to call out or ask reviewers. Explain how the changes were verified.

Note: If ADRs are introduced via a PR in GitHub, they also need to be added to SUMMARY.md in order to appear in the wiki. This PR also adds a CI check that will run when the documentation/wiki/ directory is updated in a PR and it will check that all markdown files are linked in SUMMARY.md

Here's an example of what a failure looks like

Additional information

Screenshots, GIF demos, code examples or output to help show the changes working as expected.

To see what this will look like in the wiki, you can check out this preview it should look like this:

Screenshot 2024-04-24 at 5 02 51 PM

This allows us to sync the ADRs between GitBook and GitHub
This is necessary to have them appear in GitBook UI and table of contents
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Apr 24, 2024
Copy link
Collaborator

@sumiat sumiat left a comment

Choose a reason for hiding this comment

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

PR looks good to me!
Just to make sure, for the description you stated below, you are adding it to the wiki and it will remain in github? we're not removing it from github?:

Moves documentation/decisions/ to documentation/wiki/decisions/

I think we just need a couple follow up things to make sure we're aligned on the process. These do not block approval of the PR:

Note: Future ADRs should probably default to being drafted in GitBook. If ADRs are introduced via a PR in GitHub, they also need to be added to SUMMARY.md in order to appear in the wiki.

  • That's easy to miss so we'll want to make it clear to the team that we're creating the ADRs in GitBook now.
  • Could you remind me @widal001 how often we're syncing the wiki to github?

@acouch
Copy link
Collaborator

acouch commented Apr 25, 2024

Didn't know this was possible! Very cool.

If ADRs are introduced via a PR in GitHub, they also need to be added to SUMMARY.md in order to appear in the wiki.

I would like to check w/ the team about this. The PR process has worked for us so far. We could use adrlog to generate the index so it happens automatically.

Checks that all wiki files are added to documentation/wiki/SUMMARY.md
@widal001
Copy link
Collaborator Author

widal001 commented Apr 26, 2024

@sumiat @acouch Wanted to update y'all on some changes I made to this PR and answer some of the questions above:

Updates

  • I added a new CI check that runs when a PR is opened that changes the documentation/wiki/ directory. It will fail if any of the markdown files that are added to this wiki are missing from the SUMMARY.md which is useful outside of just the new ADR workflow since it will also allow us to lint PRs opened by open source contributors that touch the wiki.
  • Here's an example of a failure which lists the missing file and the source commit in which I removed that file to trigger the failure
  • Now that we have all of the ADRs added, folks will only need to add individual ADRs to the SUMMARY.md file and this check will fail if they forget to do that

Other notes and responses

Could you remind me @widal001 how often we're syncing the wiki to github?

The wiki and GitHub get synced every time a new GitBook change request is merged and every time a GitHub PR is merged that contains code which modifies documentation/wiki/

I would like to check w/ the team about this. The PR process has worked for us so far. We could use adrlog to generate the index so it happens automatically.

Even if we use adrlog to generate the index, we still need to add new files to documentation/wiki/SUMMARY.md for GitBook to pick them up. The CI check that I mentioned above will fail when this doesn't happen so we should be able to catch it in the PR, but we can also explore options for automating the process of inserting new links as well down the line if manually adding them when a new ADR is created becomes burdensome.

sumiat
sumiat previously approved these changes Apr 26, 2024
Copy link
Collaborator

@sumiat sumiat left a comment

Choose a reason for hiding this comment

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

Thanks for the updates, Billy! LGTM. Not sure if Aaron has more questions!

sarahknoppA6
sarahknoppA6 previously approved these changes Apr 29, 2024
Copy link
Collaborator

@sarahknoppA6 sarahknoppA6 left a comment

Choose a reason for hiding this comment

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

Wow, this looks great! These changes will be really helpful. Thank you very much for putting them together and for the thorough documentation around them :)

@widal001 widal001 dismissed stale reviews from sarahknoppA6 and sumiat via ab30001 May 2, 2024 20:49
Copy link
Collaborator

@sumiat sumiat left a comment

Choose a reason for hiding this comment

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

Approved!

@widal001 widal001 merged commit 80313c9 into main May 2, 2024
11 checks passed
@widal001 widal001 deleted the issue-1820-sync-adrs-to-wiki branch May 2, 2024 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/cd documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move ADRs into documentation/wiki/ to sync with public wiki
4 participants