-
Notifications
You must be signed in to change notification settings - Fork 2.1k
doc: Integrate Governance, Code of Conduct and C++ Tutorial into Starlight #21646
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
base: master
Are you sure you want to change the base?
Conversation
doc/doxygen/Makefile
Outdated
@@ -34,7 +34,7 @@ DOXYGEN ?= doxygen | |||
DOXYGEN_CUR_VERSION = $(shell $(DOXYGEN) --version | cut -d ' ' -f1) | |||
|
|||
# for the `doc-ci` target, we can choose which Doxygen version should be built. | |||
# If nothing has been chosen, select the minimum version. | |||
# If nothing has been chosen, select the minimum version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why thic change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oopsie doopsie
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
doc/.gitignore
Outdated
guides/general/CODE_OF_CONDUCT.md | ||
guides/general/GOVERNANCE.md |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps it would make sense to create a directory that is more obvious for auto generated files. Such as "guides/generated/...". Then you can also add the whole directory to the .gitignore
instead of individual files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds like a good idea will do
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One downside of this is that the slug shown in the browser becomes http://localhost:4321/generated/governance/
instead of http://localhost:4321/general/governance/
.
From a users perspective I feel like governance being under general
makes more sense but programmatically http://localhost:4321/generated/governance/
is ofc also correct
doc/starlight/Makefile
Outdated
|
||
.PHONY: integrate_outside_files | ||
integrate_outside_files: code_of_conduct governance | ||
|
||
.PHONY: code_of_conduct | ||
code_of_conduct: ../../CODE_OF_CONDUCT.md | ||
-@echo "Generating Code of Conduct documentation..." | ||
-@sed '1,2d' $< | cat ./templates/CODE_OF_CONDUCT.template.md - > ../guides/general/CODE_OF_CONDUCT.md | ||
|
||
.PHONY: governance | ||
governance: ../../GOVERNANCE.md | ||
-@echo "Generating Governance documentation..." | ||
-@sed '1,2d' $< | sed '/<!-- TOC start -->/,/<!-- TOC end -->/d' | cat ./templates/GOVERNANCE.template.md - > ../guides/general/GOVERNANCE.md |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that using sed
in a Makefile is the best option to include the files here. A preferrable way would be to do the manipulation right in the respective .mdx files, as Astro seems to have an embedded JavaScript scripting interface that allows you to manipulate the included files:
https://docs.astro.build/en/guides/markdown-content/#dynamic-jsx-like-expressions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was simply following the way it was done in the doxygen Makefile, technically it should be possible in javascript but the complexity outweighs the usage since we would have to create our own content collection which really isnt worth it for this scenario
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a lazy maintainer and asked ChatGPT:
---
title: Governance
description: This document outlines the governance structure of RIOT.
import fs from 'node:fs'
import path from 'node:path'
/* read external Markdown file */
const markdownPath = path.resolve('../../GOVERNANCE.md');
let content = fs.readFileSync(markdownPath, 'utf-8');
/* remove the TOC block */
content = content.replace(/<!-- TOC start -->[\s\S]*?<!-- TOC end -->/, '');
---
import { Markdown } from 'astro/components';
<Markdown content={content} />
Doesn't look terribly compilcated but I didn't check whether it works or not. But hey, when was the last time AI didn't propose a working solution? 🤣
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh you wanted to do it like this, hmmm, I personally really dont like that approach because Id also cause the document to be garbage when reading on github while the current approach would have simply not shown the document on github, if anything I'd have gone with https://docs.astro.build/en/guides/content-collections/ (which is what I was referring to in my comment as too complicated for 2 files)
However, I don't have high stakes in this, I'm alright with replacing the makefile command with this if this is the preferred solution 🫡
doc/guides/c_tutorials/using_cpp.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Long lines :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The length of my ancestors (30 year olds) lines weighs on me 😔 Will fix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I trimmed the hedge
Contribution description
This integrates the Governance and Code of Conduct files into the starlight output. It should be noted that it does not touch the doxygen output and simply copies the way we do it within the doxygen output (Copy and modify the file when make gets called).
I also moved the C++ tutorial since that would mark the completion of moving all (first party) language related things into starlight. Though I wasn't sure which date to deprecate the old text so I left it TODO for now :)
Testing procedure
A simple
make doc
for doxygen should showcase the deprecation of the c++ guide. (and no changes to coc/governance)A simple
make doc-starlight
should showcase that Governance and COC are now part of the general section and C++ under the c_tutorials/ sectionIssues/PRs references
#21575