[API] Remove exported elements - #6954
Conversation
Additionally, some types only used by the unused values are also removed, and the module `OpamCmdliner.Term.Syntax` is not exported anymore because it is only composed of unexported values.
Go one step farther by replacing the use of `OpamHash.sha512` (and `OpamSHA.sha1_string`) by ``OpamHash.compute ~kind:`SHA512`` (resp. ``OpamSHA.hash_string `SHA1``) for consistency.
Additionally, remove `OpamCudf.Json` because none of its exported content is used.
Additionally, remove `src/state/shellscripts/prompt.sh`. This script is only read to create an unused value in `OpamScript`.
|
Un-removed values that are used by dependencies:
The first 3 values are related to failures in the CI. The 4th was discovered by re-verifying uses of removed values from There is an unbound value error in Depends-Linux jobs that comes from a change in #6827. I am not sure what to do of it. |
|
Hi, thanks for this experiment and maintaining dead_code_analyzer, it's been extremely useful on a local wip branch to extract what i need from a library. However i'm not sure about this PR. The opam codebase, while mainly used for the tool itself, is also exported as libraries so having unused functions is part of the job description. During development, very few of these type of functions actually ever need to be updated or causes us problem and if some do we usually realise it's not used and sometime remove them individually if they don't make sense anymore. Our benchmark also seems to be showing that there isn't much to gain by removing these unused functions but i'm tempted to keep this PR open for a bit to have time to go through it and pick-and-choose the functions and values that do not make sense anymore. |
|
I understand. Thanks for the feedback. To help you pick-and-choose, you may want to first have a look at
I suppose that, for the same reason, you would not be interested in a PR regarding optional arguments always/never used ? I might still experiment them on opam at some point to explore limits of the analyzer and write a report, but would not triage the results further then ;) |
Optional arguments could be interesting to pick-and-choose too so feel free to open it if you're planning to also do it anyway, there is no problem with keeping a PR open or opening it in the first place. Thanks again! |
Clean up unused exported values, fields and constructors discovered using the dead_code_analyzer.
I was able to confirm that the removed elements are not used outside opam (in open source projects) by using Sherlocode and exploring the project's history. The following projects have been discarded from the search for uses outside opam because they are archived or have not been active for years: marracheck, opamfu, opam-build-revdeps, opam-lock, opam-package-upgrade.
Overall 366 elements have been cleaned up (71.5% of the elements reported by the analyzer).
In addition :
src/client/opamConfigCommand.mlline 663;user_actionhas been silenced insrc/state/opamSwitchState.mlline 982;src/state/shellscripts/prompt.sh);OpamHashandOpamSHAAPIs have been simplified (a6ed337)For more details about the process, a report is available here. You might mostly be interested in the final Results, the Methodology, and the "Informed cleanup" subsections for each component in the Detailed cleanup report.
I kept each commit focused on a single component (or subcomponent) and a single analyzer report section.