-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Remove ocamldoc's literate
and todo
generators
#12615
Conversation
This commit removes the following no longer used targets and their associated recipes from ocamldoc/Makefile: dot, test, test_stdlib, test_stdlib_code, test_latex, test_latex_simple, test_man, test_texi and autotest_stdlib.
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 looks fine to me. There is a special pleasure for PRs that only remove lines of code.
I was not sure what you mean by "generators" (surely the ocamldoc backends are necessary to keep around). I looked. You mean the modules in |
Gabriel Scherer (2023/09/29 03:03 -0700):
@gasche approved this pull request.
thanks.
This looks fine to me. There is a special pleasure for PRs that only
remove lines of code.
Yeah, I do share this one.
Do you have an opinion on how far we should go, though?
|
Gabriel Scherer (2023/09/29 03:09 -0700):
> given that ocamldoc's generators are not installed, one
may wonder whether it's useful to build them and then, if not, whether they
shouldn't simply be deleted, too
I was not sure what you mean by "generators" (surely the ocamldoc
backends are necessary to keep around). I looked. You mean the modules
in `generators/` in the source tree, which apparently are one
alternative backend/plugin to generate a TODO list, around 500 lines
of code in total.
Yes, sorry for the lack of clarity here, I took it for granted that
everybody (except me before having to deal with ocamldoc's build system)
would knnow.
If they are not installed nor used in the manual
build process (I don't think they are), then yes it would be fine to
remove them.
Okay, I'll add another commit doing that properly.
|
Just pushed one more commit removing the generators completely. If that's fine with people I'll edit the PR's title and add a Changes entry. |
The |
The CI fails due to a known-flaky test,
Yes, please. |
literate
and todo
generators
PR title updated and Changes entry added |
This variable was used in ocamldoc/Makefile, by the test targets that got removed in PR ocaml#12615.
This variable was used in ocamldoc/Makefile, by the test targets that got removed in PR ocaml#12615.
This variable was used in ocamldoc/Makefile, by the test targets that got removed in PR ocaml#12615.
(This is submitted in preparation for the merge of
ocamldoc/Makefile
intothe root
Makefile
, of which a provisional implementation can befound
here.)
It is @Octachron's belief that the targets and recipes this PR proposes to
remove have not been used for the last decade or so. If they are
however considered useful, onemay want to work on integrating them
to the testsuite somehow, but since this will represent some work let's
first decide whether they are actually useful or not.
Also, if we follow the removal road, then the question of how far we should
go arises. Indeed, given that ocamldoc's generators are not installed, one
may wonder whether it's useful to build them and then, if not, whether they
shouldn't simply be deleted, too.
(For the moment I'll use a no-changes-entry-needed label but I can
definitely add a proper Changes entry if the PR is ocnsidered useful)