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

Documentation refactor #246

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Documentation refactor #246

wants to merge 4 commits into from

Conversation

oldmanauz
Copy link
Contributor

This PR was originally just supposed to update the Samples documentation as per comments in #244. Some mild refactoring happened, hopefully nothing too outrageous. Changes:

  • Made "Samples" it's own item in the documentation tree view
  • Update samples.md with the changes from Examples documentation #244
  • Added Tutorial_Memory to samples.md
  • Modified /docs/CMakeLists.txt so that the treeview item order could be manual set
  • Doxygen config file was named "html.cfg". This isn't very informative to it's function, so I renamed it to "doxygen.cfg". CMake files updated
  • Docs previously were built to "api" folder which I didn't think was overy expressive. I renamed it to "APIDocumentation". (For reference, changes in (docs/CMakeLists.txt & doxygen.cfg.in)
  • Added headings to the Manual contents page
  • Renamed some of the titles of pages in the manual to better describe what is in them
  • Renamed some of the *.md files to improve consistency.
  • Cleared all but one warning in the doxygen.log when building documents. Last remaining warning is to do with "graph_legend.dot" but couldn't work that one out.

Note: #233 will conflict with this. I'll finalise that PR so you can merge that one, then merge this after.

@oldmanauz
Documentation refactor
6dc7a04 
This PR was originally just supposed to update the Samples documentation as per comments in OGRECave#244. Some mild refactoring happened. Changes:
- Made "Samples" it's own item in the doc tree view
- Update samples.md with the changes from OGRECave#244
- Added Tutorial_Memory to samples.md
- Modified /docs/CMakeLists.txt so that the treeview item order could be manual set
- Doxygen config file was named "html.cfg". This isn't very explicit, so I renamed it to "doxygen.cfg". CMake files updated
-  Docs previously were built to "api" folder which I didn't think was overy expressive. I renamed it to "APIDocumentation". *For reference, changes in (docs/CMakeLists.txt & doxygen.cfg.in)*
- Tidied up the Manual contents page
- Renamed some of the headings in the manual and renamed the MD files to better show what is in them and to improve consistency.
- Removed all but one warning in the doxygen.log when building documents. Last warning is to do with "graph_legend.dot" but couldn't work that one out.
@oldmanauz
Copy link
Contributor Author

So I think renaming the doxygen html.cfg file caused the checks to fail. Revert this particular change?

@darksylinc
Copy link
Member

We have a script doxygen_remove_files.py that failed.

Its purpose is to remove all the junk produced before uploading to ogrecave.github.io/ogre-next/api/
Getting that script right was a pita because we can't repro it locally

Maybe it's best to revert that one for now, plus it might conflict with the name change PR

@oldmanauz
Renamed doxygen.cfg to html.cfg to fix github check
84854a9 
Github check was failing under the `Generate Doxyfile` routine with the output:
> Run cat CMake/Templates/html.cfg.in | sed 's/\${\(.*\)}/$(\1)/' > Doxyfile
cat: CMake/Templates/html.cfg.in: No such file or directory

I think this cause no doxygen files to generate then when the `Remove files` routine was run, there was nothing to remove, so it threw an error. ??
@oldmanauz oldmanauz mentioned this pull request Apr 3, 2022
Merged
@darksylinc
Copy link
Member

darksylinc commented Apr 3, 2022 •
edited
Loading

This PR renames "What's new in Ogre 2.2" into "What's new in Ogre 2.3".

I specifically don't want want to do that. The "What's new" should be preserved for historical reasons. i.e. features that were introduced in 2.2 should not be marked as introduced in 2.3

RIght now we have 3 "What's new pages". One for 2.2, another for 2.3 and another for 2.4

I don't have a problem "What's new" pages get moved into their own subpage for browsing historic records, but they should be kept separate and without "reversioning".

Though the latest changes (i.e. right now it's What's new in 2.4) should be on the main page for easy access.

@oldmanauz
Copy link
Contributor Author

@darksylinc
1.

This PR renames "What's new in Ogre 2.2" into "What's new in Ogre 2.3".

I have reverted this change in my PR. I want to tidy up the "Whats New" section but want this PR merged first so it doesn't get out of control.

  1. Just to confirm, are you happy with the documentation output path change to ".../APIDocumentation" ?

@darksylinc
Copy link
Member

Just to confirm, are you happy with the documentation output path change to ".../APIDocumentation" ?

It's probably going to raise a few headaches but I've also been bothered by the fact that there is a folder named "api" and it has nothing to do with code or interfaces at all. It's just documentation.

@oldmanauz
Copy link
Contributor Author

@darksylinc The one head ache I could think of is it might have issues with the script that updates the documentation at ogrecave.github.io/ogre-next/api/latest.

Is there a better name than "APIDocumentation"?

@oldmanauz
Copy link
Contributor Author

@darksylinc can I nudge you on this please?

@oldmanauz
Copy link
Contributor Author

@darksylinc Can I nudge you on this please? If your not interested, let me know and I'll close it out.

@darksylinc
Copy link
Member

Apologies for taking so long.

Now that I have a fresh head, I understand what was annoying me about this PR.

This PR moves many files too much. The problem is that it will break a lot of stuff:

  • Github Pages will likely start failing at first and will need fixing (fixable)
  • External Links to docs will be broken
    • We should try to avoid breaking existing documentation unless there's a good reason (e.g. the interface changed completely).
    • Doxygen by its very nature tends to unfortunately break links (because random%%GenErated_name in Doxygen version 123 becomes random__GenErated+name in Doxygen 456. But we should avoid doing it on purpose.
    • Moving Samples to its own folder is acceptable because this section is very new. There aren't many external links (if any) to it yet
    • manual cannot be renamed to Manual

But there are many other changes that are welcome:

  • Typo fixes
  • Syntax fixes
  • The new heading organization in manual.md
  • Heading changes (e.g. # becoming ##)

These changes are welcomed but are more controversial:

  • Changes in Samples.md
  • Moving samples' png files to its own folder

Therefore I suggest splitting the "welcomed non-controversial" changes into its own PR (which can be accepted immediately) and put leave the controversial changes in this PR (or a new one).

@eugenegff eugenegff force-pushed the master branch 2 times, most recently from 3904aa1 to da6dbf9 Compare October 7, 2022 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@oldmanauz @darksylinc