Skip to content

Conversation

@tenzap
Copy link
Contributor

@tenzap tenzap commented Jul 7, 2025

Same as #453 but this one keeps the html files (and doesn't delete them in the clean task) although I don't really see why they should be kept since:

  • if the generated file doesn't exists it is created anyway by the servlet
  • if the file is older than the template it will be regenerated by the servlet
    However, in both cases, this requires the servlet to have write permission on their folder.

I can suppose this can be useful for jigsaw, and maybe as deployed war (on tomcat/jetty...), but this reveals bug #451

Checking whether regeneration is required using the timestamp doesn't really make sense in a git repo as the timestamps of the commit are not kept&restored from the commit date IIRC. But on a deployed app using the timestamp makes sense.

Also, eg. on tag cssval-20250226, the generated files were not in line with the template. Maintaining template & generated files synchronized is extra work while having them generated ensures that they match the template.

IMHO, the benefit of keeping the HTML files would be that one can see how a change in the template impacts the generated HTML.

You may merge this PR or the previous one #453 , as you prefer, but I think it may be better to remove the generated html and keep them generated at build time + 1st load in the case the deployed app doesn't ship them. That's not much extra work and actually permits to ensure that the generation works before deployment, instead of discovering after deployment that there is a generation issue.

@ylafon
Copy link
Member

ylafon commented Jul 8, 2025

Thanks!

@ylafon ylafon merged commit 4d612a6 into w3c:main Jul 8, 2025
1 check passed
@tenzap
Copy link
Contributor Author

tenzap commented Jul 8, 2025

Could you write a few words to elaborate what was the probmem with removing the html? It would be helpful in better understanding the rationale or what I actually misunderstood.

@tenzap
Copy link
Contributor Author

tenzap commented Jul 8, 2025

Given that I remove them in the Debian package I'd like to be sure that it wouldn't cause a problem, and what I would have missed that you know of.

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.

2 participants