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

Build man page as part of workflows #149

Merged
merged 2 commits into from
Feb 16, 2025

Conversation

blairconrad
Copy link
Contributor

@blairconrad blairconrad commented Feb 10, 2025

Fixes #74.
Closes #149. (Which it includes.)

@blairconrad
Copy link
Contributor Author

blairconrad commented Feb 10, 2025

Happy to change anything (especially dedup once you're otherwise happy, if the repetition offends).

You can see recent workflows:

Note: there are a fair number of warnings, but I don't think my changes made them qualitatively worse. Probably an extra "The set-output command is deprecated...", but this appears to be coming in from actions that are already being used. Likely upload-artifact, but it's just a guess.

Also, release 2.0.5 was created from those builds.

Oh, one quite disappointing thing (for me) is that the man page generation is VERY slow. Mostly in staging the environment. An asciidoctor docker image exists that on my local machine gave much faster results (once I pulled the imager the first time; don't know if that'd be an issue for the GitHub Actions runner), if the (lack of) speed is a concern. The output was nearly the same as that for the existing build process, and could probably be closer with minor massaging of the git-absorb.txt input.

@blairconrad blairconrad marked this pull request as draft February 10, 2025 03:06
@@ -1,5 +1,5 @@
:man source: git-absorb
:man version: 0.5.0
:man version: {man-version}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

couldn't figure out how to supply a value directly from the command line, so aliased the attribute

@blairconrad blairconrad marked this pull request as ready for review February 11, 2025 02:44
@blairconrad
Copy link
Contributor Author

Okay, that's all I have. If the changes can better fit your vision, do let me know.

@tummychow tummychow self-assigned this Feb 11, 2025
@blairconrad blairconrad mentioned this pull request Feb 12, 2025
Copy link
Owner

@tummychow tummychow left a comment

Choose a reason for hiding this comment

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

works for me. the flag should be an easy speedup, but otherwise i don't care too much about how long it takes to run in ci

- name: Setup environment
run: |
sudo apt-get update
sudo apt-get --assume-yes install asciidoc
Copy link
Owner

Choose a reason for hiding this comment

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

could you add --no-install-recommends? probably part of the reason it's slow is it's installing texlive, which has tons of heavy binary assets in it (eg fonts)

@blairconrad
Copy link
Contributor Author

blairconrad commented Feb 16, 2025

Rebasing to resolve conflicts (and make viewing subsequent diffs easier).

@blairconrad blairconrad force-pushed the build-man-page branch 2 times, most recently from 44d2ef6 to 1f41c62 Compare February 16, 2025 12:42
@blairconrad
Copy link
Contributor Author

blairconrad commented Feb 16, 2025

I opted to incorporate the change instead of

** OMG that was fast! build-man-page beat any of the test runs! I didn't even have time to finish taht comment about the fixups I was making and will continue now! **

pushing the fixup and squashing later, since it's so simple and you can see it in the Compare link anyhow.

@blairconrad
Copy link
Contributor Author

Whoops. Forgot release.yml. Fixed.

@blairconrad blairconrad mentioned this pull request Feb 16, 2025
@tummychow tummychow merged commit 60a8f2c into tummychow:master Feb 16, 2025
5 checks passed
@blairconrad blairconrad deleted the build-man-page branch February 17, 2025 00:26
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.

man page contains wrong version number
2 participants