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

List instead of table #327

Merged
merged 4 commits into from
May 19, 2024
Merged

Conversation

yakatz
Copy link
Contributor

@yakatz yakatz commented May 14, 2024

Based on (includes) #262, adds additional ways of seeing the included facts

@yakatz yakatz force-pushed the list_instead_of_table branch 2 times, most recently from e152fbd to 6e64aaa Compare May 16, 2024 18:56
@yakatz yakatz marked this pull request as ready for review May 16, 2024 18:56
@@ -17,6 +17,8 @@ jobs:
ruby-version: '3.3'
env:
BUNDLE_WITHOUT: release
- name: Generate database
run: bundle exec rake database
Copy link
Member

Choose a reason for hiding this comment

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

should we update it on each release or on each merge?

Copy link
Member

Choose a reason for hiding this comment

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

mhm what does that actually do? just build a yard db that will be added to the gem before it's uploaded?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It builds the old table from the README and several lists sorted in different ways. @ekohl had added it to the .gitignore in PR #262, so I kept it that way, meaning this would be run on release. Does it make sense to run on every merge instead and remove from the .gitignore? Run on every merge and just update GH pages?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

I'm a big fan of not including generated files in git, so will generally approve PRs that avoid it. That said, if this job doesn't show it then I assume it's verify the job passes. Is that correct?

@yakatz yakatz force-pushed the list_instead_of_table branch 4 times, most recently from 432ac5e to 0b44356 Compare May 16, 2024 20:12
@bastelfreak
Copy link
Member

I think this is fine for now and we should merge it so it lands in the next major release. We can always improve it afterwards. @yakatz can you rebase and merge afterwards?

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

Not a complete review, just some things that stood out to me.

path: doc
- name: Deploy to GitHub Pages
id: deployment
uses: actions/deploy-pages@v1
Copy link
Member

Choose a reason for hiding this comment

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

Latest version is v4

Suggested change
uses: actions/deploy-pages@v1
uses: actions/deploy-pages@v4

- name: Set up Pages
uses: actions/configure-pages@v2
- name: Upload artifact
uses: actions/upload-pages-artifact@v1
Copy link
Member

Choose a reason for hiding this comment

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

Latest is v3

Suggested change
uses: actions/upload-pages-artifact@v1
uses: actions/upload-pages-artifact@v3

- name: Build docs
run: bundle exec rake yard
- name: Set up Pages
uses: actions/configure-pages@v2
Copy link
Member

Choose a reason for hiding this comment

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

v5 is latest

Suggested change
uses: actions/configure-pages@v2
uses: actions/configure-pages@v5

@@ -17,6 +17,8 @@ jobs:
ruby-version: '3.3'
env:
BUNDLE_WITHOUT: release
- name: Generate database
run: bundle exec rake database
Copy link
Member

Choose a reason for hiding this comment

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

I'm a big fan of not including generated files in git, so will generally approve PRs that avoid it. That said, if this job doesn't show it then I assume it's verify the job passes. Is that correct?

ekohl and others added 3 commits May 18, 2024 22:43
This changes the workflow to have a rake task to create a database.md
file and then render that into the generated documentation.

The release process is also enhanced to include the generated
database.md file. This should allow sites like rubydoc.info correctly
render it as well, though this hasn't been tested.

It also introduces a GitHub Action workflow to build GitHub Pages with
the generated documentation. That means that the documentation should
always be up to date and no manual steps are needed anymore. It also
saves a lot of noise in the git log.
@yakatz yakatz merged commit e570582 into voxpupuli:master May 19, 2024
7 checks passed
@yakatz yakatz deleted the list_instead_of_table branch May 19, 2024 02:45
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