-
Notifications
You must be signed in to change notification settings - Fork 2
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
Support devcontainer, refactor GitHub acitons, add documentation #92
Conversation
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.
Really cool stuff, and save me some red tape so nice work.
Ready for review and also hopefully merging soon! |
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.
This is great PR with much needed changes. Only minor tweaks needed.
- If we are removing deprecated options in Doxyfile.in, we should remove the descriptions of those options. Otherwise, it may be too confusing for a maintainer who may look at this a few months later.
- Do we have instructions somewhere how to build documentation in the docker? That would be helpful for other developers.
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.
Some minor even inconsequence suggest, but overall still great.
e4f1a35
to
b68b9cf
Compare
b68b9cf
to
4c99dff
Compare
@pelesh I think this is good to merge. I added instructions for running the dev container in index.rst. LGTM |
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.
Looks good! The only missing thing is to restore doxygen-awesome stylesheet.
I get following error when I try to create a container:
@cameronrutherford @ryandanehy: Do you know what this means? |
Are you on VPN? |
Yes, I was. I'll try w/o VPN. |
Implements:
Closes:
As well as documenting new features added in this PR
@ryandanehy