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

Replace node config reference with code generation. #1183

Merged
merged 7 commits into from
Oct 25, 2023

Conversation

winder
Copy link
Contributor

@winder winder commented Sep 26, 2023

With algorand/go-algorand#5729 the node config inline documentation is brought into alignment with the developer portal.

This PR uses the config_json_gen utility to render the node config reference page from the code comments.

@winder winder self-assigned this Sep 26, 2023
@winder winder marked this pull request as draft September 26, 2023 15:30
@winder
Copy link
Contributor Author

winder commented Sep 26, 2023

Converted back to the draft. It doesn't seem to display properly:
https://github.com/algorand/docs/blob/bc19457bd4f5340b8b9b2f57ac756e5665e8cd44/docs/run-a-node/reference/config.md

@winder
Copy link
Contributor Author

winder commented Sep 26, 2023

@winder winder marked this pull request as ready for review September 26, 2023 15:42
ryanRfox
ryanRfox previously approved these changes Sep 27, 2023
Copy link
Contributor

@ryanRfox ryanRfox left a comment

Choose a reason for hiding this comment

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

looks like it renders properly now. good stuff here

@nullun
Copy link
Collaborator

nullun commented Oct 4, 2023

Just came across this after manually updating the page in #1186 🤦

@winder
Copy link
Contributor Author

winder commented Oct 4, 2023

Just came across this after manually updating the page in #1186 🤦

Ouch. Did the automation do a good job? How do we get the process updated?

@nullun
Copy link
Collaborator

nullun commented Oct 4, 2023

The script works great! This is far better, thanks Will.

I think ultimately we want it to be run as part of the release-checker pr-generator workflow so when a new stable release is made it'll PR any new config.md changes, but I'm not familiar with that process atm.

I'm happy to approve this PR and get it merged in though.

Copy link
Collaborator

@nullun nullun left a comment

Choose a reason for hiding this comment

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

@winder I added a single function to the code to HTML encode the default values, I have also updated the pr-generator to run this on new releases. Look good?

@ryanRfox Could I get you to take a last look to make sure there's nothing obviously wrong?

Then I'll merge.

@winder
Copy link
Contributor Author

winder commented Oct 25, 2023

The recent release includes some new data directory configurations. It would be good to wrap up this PR, and run the script again to pull in the new configurations.

cc: @nullun @ryanRfox

Copy link
Collaborator

@nullun nullun left a comment

Choose a reason for hiding this comment

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

Just tested again locally and am all for merging it in. @ryanRfox ?

@ryanRfox ryanRfox merged commit 9d0dc7c into staging Oct 25, 2023
1 check passed
@winder winder deleted the will/generated-config branch October 25, 2023 17:25
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.

3 participants