-
Notifications
You must be signed in to change notification settings - Fork 408
docs(make): improve and centralize make help output #4265
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?
docs(make): improve and centralize make help output #4265
Conversation
🛠 PR Checks SummaryAll Automated Checks passed. ✅ Manual Checks (for Reviewers):
Read More🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers. ✅ Automated Checks (for Contributors):🟢 Maintainers must be able to edit this pull request (more info) ☑️ Contributor Actions:
☑️ Reviewer Actions:
📚 Resources:Debug
|
This is still an early proof of concept. I welcome feedback. while I might have liked doxygen style comments for each target, this implementation is significantly less complex than would be required for multiple-line prefixed comment block parsing. |
FYI: work not yet finished: comments should be added to all relevant targets |
The makefile looks too complex. |
You raise a valid point. I’ll admit I may be a bit too comfortable with complex Makefiles for my own good. I’ll likely finish this PR regardless—as a learning exercise—but I was hoping it might prove useful to others as well. For the sake of discussion, here’s a side-by-side comparison of the output before and after this change, using the two directories I’ve completed so far: Before:
After:
That said, this might be too much information for a new contributor at once. @moul – I appreciate you taking a look. I agree this is more complex than ideal, and I’m still not convinced the value justifies the overhead. There’s also quite a bit of boilerplate here that could probably be factored out into a shared script to simplify the Makefiles. Either way, I look forward to wrapping this up and turning my attention to a project with more lasting impact. |
Hi @moul, I'm not sure if this is quite what you had in mind. The logic is essentially the same as before, but now centralized in It does introduce light use of Make functions, which might make it a bit harder to read at first glance, but in exchange, all of the comment scraping and formatting logic now lives in one place. That said, if you feel this direction complicates things more than it helps, I’m happy to revisit or revert it. Best regards, P.S. I still have to add the applicable comments to: |
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
Hi @loren-osborn . Can you please fix the failed check for "PR Title Linter"? |
Hi @jefft0 — thanks! I’ve updated the title to use docs, since this improves make help output and centralizes related logic. If anyone feels it fits better under build, I’m happy to adjust. |
Hi @loren-osborn . The "PR Title Linter" is still unhappy. "WIP" is usually for a draft PR. Is the PR ready for review? If yes, then please remove "WIP". But if you are still working on it, then please convert to draft. |
Yes, while I'm close to done, it is still WIP... (I would rather mark this with a label, but I don't believe I have labeling permission) |
I marked it as a draft in the GitHub UI. You should also be able to change it; you can find a link to "Convert to draft" on an issue sidebar, and you can then click "ready for review" in the merge box when it's ready. |
Also added README.md scraping for sub-makes
Thanks, @thehowl! I believe the PR is now ready for review. I wasn't able to find a "GitHub UI" application specifically, but I did install GitHub Desktop. From there, selecting the PR just opens it in the browser—but I did go ahead and mark it as "ready for review" from there. Thanks again! |
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.
Thanks for your contribution :)
I think this is in a good spirit; but there's a bit too much logic being written as make rules for my taste; I don't understand what's going on in makeHelpMsgs.mk
, and I don't want to.
Can we make a makefile_help.sh
script, which given an argument of a makefile, generates the desired help output?
I believe bash scripts are generally easier to read and maintain than makefile magic.
contribs/gnohealth/README.md
Outdated
|
||
### Run the default health check (timestamp): | ||
|
||
=== |
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.
use ```
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.
Good catch.
That makes sense—thanks for the feedback! I’ll go ahead and move the logic into a The
To match the current behavior, I’ll pass in the Makefile, the |
This would have been rather unmaintainable if completed... switching to Go.
@thehowl I have to head out shortly and still have quite a bit to do to get this production-ready, but I wanted to check if you’d be willing to take a quick look at
I won’t mark this "ready for review" until it’s cleaned up—I’m just hoping to get some early feedback. P.S. I'm available on Discord if you want to discuss this at all. P.P.S. I’m not a huge fan of the |
Getting close to review ready...
|
I'm sure there's something I forgot to fix, but I took care of all issues that I was aware of. I computed and passed all the files and directories as arguments from make, but all the file parsing happens in Go now. |
Summary
This PR provides a minor quality-of-life improvement for new Gno developers by enhancing
make help
output across the project.Highlights
make help
now displays inline comments appended to target lines, offering clear, one-line explanations for each Make target.make -C contribs help
:make help
outputs,README.md
to display a brief description of each tool.No functional changes—just improved discoverability and documentation for contributors.