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

Detecting and deleting dead scripts #1538

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dldisha
Copy link

@dldisha dldisha commented Apr 16, 2021

Issue Description

There are a couple of scripts in the codebase that seem to be dead code and don't have an impact on the development.

Fixes #1531

Changes Made

I have deleted all such scripts which are not being used for the development anymore. For resolving the issue, I first removed one script at a time and build the project again. If the build was successful without the script then that particular script is not being used for the development and is a dead code.

In total, I detected 7 such scripts that are dead code.

  • Please check if the PR fulfills these requirements
  • PR is descriptively titled and links the original issue above
  • Before/after screenshots (if this is a layout change)
  • Details of which platforms the change was tested on (if this is a browser-specific change)
  • Context for what motivated the change (if this is a change to some content)

Copy link
Contributor

@patricoferris patricoferris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR.

I think you've removed a little too much :)) For example:

  • graph.ml and test.ml are both linked in the gtk tutorial so they have to stay.
  • similarly with objcache.ml only in the garbage collection tutorial.
  • absolute_urls this one appears to be used in Makefile.opam.ocaml.org, I'm not entirely sure of the deployment process for opam.ocaml.org, but if it does still use this file then we probably shouldn't be removing it either.
  • link_blog_doc_ext.sh is also an opam.ocaml.org one which I would be fearful of removing

Which only leaves ocamlapplet.bash which quite possibly could be removed, although it would be great if you could provide more information why. When you said:

If the build was successful without the script then that particular script is not being used for the development and is a dead code.

I think you need to dig a little deeper than this, you can try using tools like grep (or cmd+shift+f is VSCode) to search for uses of these files, try understanding what they might be used for and see if there are any uses cases. Most importantly, it would be good to relay this information in the PR itself as it will make for much quicker reviewing.

I appreciate the hard work so far, thanks :))

@dldisha
Copy link
Author

dldisha commented Apr 19, 2021

Thankyou @patricoferris for the review and the explanation. I'll study the scripts more deeply and get back to this asap.

@dldisha
Copy link
Author

dldisha commented Apr 21, 2021

grep was quite helpful :) Helped me to undersnatd a lot about the file system.

Coming to the issue, I think we may not remove ocamlapplet.bash too because it is linked to Makefile.common; a template file being used to generate certain configuration and run a certain task if something goes wrong with automatic config.

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.

Spring clean up: detect and delete dead scripts
2 participants