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.

Sign up for GitHub

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

Jump to bottom

[docs] Relative filenames. Using doxygen-awesome #507

Merged
merged 5 commits into from
Mar 12, 2022
Merged

[docs] Relative filenames. Using doxygen-awesome #507

merged 5 commits into from
Mar 12, 2022

Conversation

1uc
Copy link
Collaborator

@1uc 1uc commented Mar 4, 2022 •
edited
Loading

Improves the Doxygen generated documentation with the following features.

  • Adds the doxygen-awesome style sheet.
  • Makes filenames relative, e.g. in file dependency graphs.

Improves #472

@1uc 1uc marked this pull request as ready for review March 4, 2022 12:34
@1uc 1uc changed the title Fix issue #472 Improve issue #472 Mar 4, 2022
tdegeus
tdegeus reviewed Mar 9, 2022
View reviewed changes
Copy link
Collaborator

@tdegeus tdegeus left a comment

Choose a reason for hiding this comment

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

Awesome! I was tempted to do this already when I looked at the output in your other issue.

doc/doxygen-awesome-css/download_stylesheet.sh Outdated
@@ -0,0 +1,10 @@
#! /usr/bin/env bash
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this file here? Wouldn't it make sense to include all needed files here (that is at least what I do elsewhere)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

All files needed are included in the repository. This file can be removed.

The hope is that this file documents which files need to be downloaded to update the styling. The script provides the answer, just one, and it also provides the URL where that one file can be found (hopefully).

Would you like me to remove it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

My suggestion would be to have a GH Action check if doxygen-awesome is outdated. It could run for example on all merges to the main branch.
Ideally it could even open a PR if it founds things that are outdated, but I don't know if that is possible.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Just the first hit when searching : https://github.com/istein/gh-actions-open-pr/blob/main/.github/workflows/pr.yml
It seems like we can modify this to do what we want

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nice idea, I'll improve the PR to incorporate this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Amazing. Let me know if you need help!

@tdegeus tdegeus changed the title Improve issue #472 [docs] Relative filenames. Using doxygen-awesome Mar 9, 2022
@1uc
Add GHA workflow to check doxygen-awesome version
d6885cc 
Rename `download_stylesheet.sh` to `update_doxygen_awesome.sh`. Since
the script must check out the doxygen-awesome repo, it need to be passed
a folder that is 'safe' to pollute.

The workflow will download the most current version of the CSS. Then
check if it's different from our version. It will proceed to create a
pull request, but only if it hasn't done so on a previous run.
@1uc
Copy link
Collaborator Author

1uc commented Mar 10, 2022

I've tested the workflow in a separate repository quite thoroughly. Is there a way of testing it in this repo without effecting others adversely?

tdegeus
tdegeus requested changes Mar 11, 2022
View reviewed changes
Copy link
Collaborator

@tdegeus tdegeus left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Amazing! I like this feature!

Other than the minor comment to me it looks fine.
No, I don't know if we can pre-test it. I think we will just have to try. I will try it elsewhere too, just to verify.

.github/workflows/check_doxygen_awesome_version.yml Outdated Show resolved Hide resolved
@tdegeus
Copy link
Collaborator

tdegeus commented Mar 11, 2022 •
edited
Loading

I check too, works like a charm! https://github.com/tdegeus/GooseFEM
Thanks again, I think this is an amazing feature.
You could even consider write a GH Action for it!

1uc and others added 2 commits March 11, 2022 19:32
@1uc @tdegeus
Prefer 'ubuntu-latest' of a pinned version.
69d4452 
Co-authored-by: Tom de Geus <tdegeus@users.noreply.github.com>
@1uc @tdegeus
Add name of the workflow.
9cb8d73 
Co-authored-by: Tom de Geus <tdegeus@users.noreply.github.com>
@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@c9739ef). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #507   +/-   ##
=========================================
  Coverage          ?   78.54%           
=========================================
  Files             ?       63           
  Lines             ?     3276           
  Branches          ?        0           
=========================================
  Hits              ?     2573           
  Misses            ?      703           
  Partials          ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c9739ef...9cb8d73. Read the comment docs.

@tdegeus tdegeus mentioned this pull request Mar 12, 2022
Open
@tdegeus tdegeus merged commit d80c1f5 into master Mar 12, 2022
@tdegeus tdegeus deleted the fix_issue_472 branch March 12, 2022 10:18
@1uc
Copy link
Collaborator Author

1uc commented Mar 12, 2022

It worked :) I was not expecting it to trigger this soon. Very nice idea of yours. I'll look into turning it into an GH action. Thanks for all the feedback during review.

@tdegeus
Copy link
Collaborator

tdegeus commented Mar 12, 2022

Like a charm! Thanks again, this is a really nice feature. It would be really nice to generalise this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tdegeus tdegeus tdegeus approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@1uc @tdegeus @codecov-commenter