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

Superfluous file build_text.js.in for documentation generation. #5190

Open
albert-github opened this issue Jan 27, 2025 · 13 comments
Open

Superfluous file build_text.js.in for documentation generation. #5190

albert-github opened this issue Jan 27, 2025 · 13 comments
Labels
type:Documentation Documentation improvement or change

Comments

@albert-github
Copy link
Contributor

Description

In the repository a file buid_text.js.in exists that is probably superfluous.
Content:

$(function(){
    document.getElementById("datetime").textContent = "@_DATETIME@"
});

Expected information

Not using a locally defined date time function for the documentation but using in the header / footer $datetime and in body text use the doxygen command \showdate (available since doxygen version 1.9.5).
When the file is necessary it would be better not to place it directly in the html output directory, but in another directory in the build tree and use the setting HTML_EXTRA_FILES to copy the file to the destination folder.

Actual information

No datetime found in the repository (except for build_text.js.in)

Versions

master acaddf4

Additional Information

Saw a problem when I ran some tests and removed the html directory and afterwards ran doxygen outside of cmake / nmake (which is not the best way but quite convenient during debugging.

@albert-github albert-github added the type:Documentation Documentation improvement or change label Jan 27, 2025
@albert-github albert-github changed the title Superflouous file build_text.js.in for documentation generation. Superfluous file build_text.js.in for documentation generation. Jan 27, 2025
@albert-github
Copy link
Contributor Author

@blowekamp
Nice to give a thumb down but it would be better to give a reason for it as well!

@blowekamp
Copy link
Member

@albert-github
I have been instructed that I can publish code but not engage with the public at this time. Thank you for your understanding.

@albert-github
Copy link
Contributor Author

Well I hope that someone else will have the permission to engage with the public and can explain a thumb down.

@dzenanz
Copy link
Member

dzenanz commented Jan 28, 2025

I was also puzzled by the thumbs down without explanation. I guess it means that buid_text.js.in is not superfluous.

@dzenanz
Copy link
Member

dzenanz commented Jan 28, 2025

Maybe it is related to this? an “immediate pause” had been ordered on — among other things — regulations, guidance, announcements, press releases, social media posts and website posts until such communications had been approved by a political appointee.

@albert-github
Copy link
Contributor Author

@dzenanz
Looks like it, but as long as @blowekamp cannot share the reason for his thumbs down we will be riding in the dark.
Maybe @blowekamp has the possibility to communicate it directly to you.

@dzenanz
Copy link
Member

dzenanz commented Jan 28, 2025

No, he sent me an email with the same statement (I have been instructed...).

@dzenanz
Copy link
Member

dzenanz commented Jan 28, 2025

Let's keep this issue open until NIH lifts this (hopefully transitional and short-lived) policy.

@albert-github
Copy link
Contributor Author

A very very big pity and I hope it will soon be possible that the NIH lifts this policy.

@thewtex
Copy link
Member

thewtex commented Jan 28, 2025

Hi @albert-github ,

I think the reason @blowekamp added this file is still valid. It is explained in the associated commit message:

The date and time are not set by javascript when the document is
loaded. This removes nightly modification of all the HTML files, and
enable traditional static HTML cache to work, and enable the generated
HTML to be committed to git regularly without nightly modification.

@albert-github
Copy link
Contributor Author

@thewtex
Thanks.
Might be the case but so in other words even when the file didn't change but has been regenerated from new source and the file didn't change (in the doxygen sense) it still is changed as the value of datetime is changed.

Committing result files like HTML files to git is in my opinion bad practice (they can be as artifacts with releases in the zip / tar.gz files which should be sufficient).

I still don't see the need for it, and as written before when one wants to have a unique date in the HTML file there is still the doxygen $datetime that can be placed in e.g. the header or footer file and will put the date / time of generation in the file.

Furthermore there might be numerous other files as well, like svg, js, ... so these won't get a timestamp either.

I think it is necessary to know exactly why this js code should be present, so probably we have to wait till @blowekamp can engage with the public again.

@thewtex
Copy link
Member

thewtex commented Jan 30, 2025

@albert-github more context:

We used GitHub Pages to publish the HTML. It used to be necessary to commit to Git for GitHub Pages. Recently, GitHub provided an alternative for GitHub Pages that does not require Git commits. We did move to that recently, along with publishing to ReadTheDocs (see the ITK 5.4.0 release notes for more information on that).

However, there is still the issue of browser cache hits. With the ReadTheDocs CDN, the responsiveness of loading the documentation is greatly improved. However, we may want to keep this. I agree that we should get @blowekamp 's input before making changes.

@albert-github
Copy link
Contributor Author

FYI

  • the cache can indeed be quite usable but also quite a nuisance, though with $datetime this should also not happen (has similar functionality as the javascript
  • on github.io there is also doxygen internal documentation published, the procedure used:
      - name: Generate Internal documentation
        shell: cmake -P {0}
        run: |
          execute_process(
            COMMAND cmake --build build --target docs_internal
            RESULT_VARIABLE result
          )
          if (NOT result EQUAL 0)
            message(FATAL_ERROR "Building internal documentation failed")
          endif()
        if: matrix.config.name == 'Ubuntu Latest GCC Release'
    
      - name: Publish Internal documentation to Github pages
        uses: peaceiris/actions-gh-pages@v4
        with:
           deploy_key: ${{ secrets.ACTIONS_DEPLOY_KEY }}
           external_repository: doxygen/doxygen-docs
           publish_dir: build/doxygen_docs/html
           force_orphan: true
        if: ${{ github.event_name == 'push' && matrix.config.name == 'Ubuntu Latest GCC Release' }}
    
    (see https://github.com/doxygen/doxygen/blob/master/.github/workflows/build_cmake.yml)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:Documentation Documentation improvement or change
Projects
None yet
Development

No branches or pull requests

4 participants