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

maintainers.html: Add list of maintainers #141

Merged
merged 11 commits into from
Dec 9, 2024

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Dec 6, 2024

After trying a lot around to get a script running to add the list of maintainers to the documentation (see RIOT-OS/RIOT#21062 )1, @aabadie and I came to the conclusion that it might be best to have a dedicated service to generate the list and then somehow include the generated list into the documentation. All this generation-by-service-stuff, however, made me think: “We already have something like this: riot-os.org, our website, so why not just integrate the list of maintainers into our website?” So I did with this PR. All it takes was to change the access token for the deployment workflow.

Footnotes

  1. Overall conclusion: reading teams needs a token with special permissions, generating the list as artifacts of a dedicated GitHub actions yields the list, but Murdock [and our doc building service] would again need a special GitHub token to download them.

Copy link

github-actions bot commented Dec 6, 2024

🚀 PR preview deployed to https://RIOT-OS-riot-os-org-preview-141.surge.sh

@miri64
Copy link
Member Author

miri64 commented Dec 6, 2024

Currently only the default list of maintainers is rendered, as the token is not set for this PR (as it introduces the usage of this token). I tested it locally, but should we maybe at first not link maintainers.html in the menu and see if when merged to master the result is the correct one?

waehlisch
waehlisch previously approved these changes Dec 8, 2024
maintainers.html Outdated Show resolved Hide resolved
maintainers.html Show resolved Hide resolved
maintainers.html Outdated Show resolved Hide resolved
maintainers.html Outdated Show resolved Hide resolved
maintainers.html Outdated Show resolved Hide resolved
maintainers.html Outdated Show resolved Hide resolved
maintainers.html Outdated Show resolved Hide resolved
@miri64
Copy link
Member Author

miri64 commented Dec 9, 2024

Addressed comments, but we now need a re-ACK.

@waehlisch waehlisch self-requested a review December 9, 2024 11:12
waehlisch
waehlisch previously approved these changes Dec 9, 2024
Copy link
Member

@waehlisch waehlisch left a comment

Choose a reason for hiding this comment

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

if the subcaption is fixed, ready to go from my side. thanks @miri64!

@miri64
Copy link
Member Author

miri64 commented Dec 9, 2024

if the subcaption is fixed, ready to go from my side.

Done.

maintainers.html Outdated Show resolved Hide resolved
Truncate full name if too long, use muted styling for full name
maintainers.html Outdated Show resolved Hide resolved
Fix fetching to not stringify "None"
Copy link
Collaborator

@leandrolanzieri leandrolanzieri left a comment

Choose a reason for hiding this comment

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

Looks good from my side!

@miri64 miri64 merged commit 14d4c5d into RIOT-OS:master Dec 9, 2024
1 check passed
@miri64 miri64 deleted the maintainers-list branch December 9, 2024 13:19
@miri64
Copy link
Member Author

miri64 commented Dec 9, 2024

Thanks for the review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants