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

Add RFC book #1

Merged
merged 39 commits into from
Dec 1, 2023
Merged

Conversation

rzadp
Copy link

@rzadp rzadp commented Nov 29, 2023

  • This is a PR from a branch in paritytech-stg into a branch in paritytech in order to review the changes before sending the actual PR to fellows.
  • It generates a web page of RFCs using mdBook
  • The current preview can be seen here.
  • Progresses [EngAut] RFCs Web Page eng-automation#1

@rzadp
Copy link
Author

rzadp commented Nov 29, 2023

Note: There is a bug in mdbook that causes the printed doc to look like this if the side bar is open:

Screenshot 2023-11-29 at 11 56 46

The fix has been merged but not released yet.

Copy link

@Bullrich Bullrich left a comment

Choose a reason for hiding this comment

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

To be honest I'm not a big fan of having such a big JS script inside the yml. Is there any way to move it to a file? There is an explanation on how to do this

push:
branches:
- main
- rzadp/rfc-book

Choose a reason for hiding this comment

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

Is this intentional or a left over of testing?

Copy link
Author

Choose a reason for hiding this comment

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

Intentional for now - as long as we keep it in paritytech-stg or paritytech.
I'll remove it (and uncomment cron) before making a PR to fellows.

).data.filter(
(file) => file.status === "added" && file.filename.startsWith("text/") && file.filename.includes(".md"),
);
if (addedMarkdownFiles.length !== 1) continue;

Choose a reason for hiding this comment

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

This is to filter the markdowns? There should only be one file per PR?

Copy link
Author

Choose a reason for hiding this comment

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

Exactly, there is a process for creating the RFC PRs and here we try to filter out any PR that is not a standard RFC PR.

The patch in this object is not a full patch with valid syntax, so we need to modify it a bit - add a header.
*/
// This header will cause the patch to create a file in patches/text/*.md.
const patch = `--- /dev/null\n+++ b/patches/${rfcFile.filename}\n` + rfcFile.patch + "\n"

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

Yeah.. :D

@rzadp
Copy link
Author

rzadp commented Nov 30, 2023

To be honest I'm not a big fan of having such a big JS script inside the yml. Is there any way to move it to a file? There is an explanation on how to do this

Good call, done ✅

@rzadp rzadp requested a review from Bullrich November 30, 2023 10:18
Copy link

@Bullrich Bullrich left a comment

Choose a reason for hiding this comment

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

Looks great! It has quite a lot of interesting solutions and it is a bit tricky to read (I don't know if there is a "leaner" way to do it, but not every complex problem can have a simple solution).

The end result is also very good!

Good job!

@rzadp rzadp merged commit 1bbb7a6 into paritytech:rzadp/rfc-book Dec 1, 2023
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 this pull request may close these issues.

2 participants