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

Remove some old scripts #22534

Merged
merged 1 commit into from
Jan 17, 2022
Merged

Remove some old scripts #22534

merged 1 commit into from
Jan 17, 2022

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Jan 17, 2022

Overview

Scripts in tools directory don't actually ship - the only reason for them is if we use them in the release process I think...

Before

These scripts seem very unlikely to be used to me
NormalizePhone
soapClient
testProcess
test_sandbox

After

Listed ones removed
This handful still exists - do we use them?

image

Technical Details

@seamuslee001 unless you or @totten use these - or the remaining ones - I'd be pretty sure they are not used.

We seem to occassionally have updated them but only for style & security reasons (it was style issues that popped them onto my radar today)

Comments

@civibot
Copy link

civibot bot commented Jan 17, 2022

(Standard links)

@civibot civibot bot added the master label Jan 17, 2022
@seamuslee001
Copy link
Contributor

set-version is definitely used unsure about the others

@totten
Copy link
Member

totten commented Jan 17, 2022

Yeah, we definitely use set-version.php.

Agree with all the removals in the current PR. Can't find any evidence of them being used by universe. Additionally, the tools/ folder is generally omitted from tarballs, so the only folks who even have the chance to use/not-use them are folks coming through git...

This folder is a kind of random grab-bag -- I think everyone who looks at it says, "I don't know what these are for." Skimming these... here are a few that I'd straight-up vote for removal:

  • tools/bin/scripts/cli.php - Looks redundant with bin/cli.php (in principal) and inert (in practice - last line is commented out).
  • tools/bin/scripts/replace.sh - Became unnecessary ~1 year ago when we stopped listing the year at the top of every PHP file.
  • tools/bin/scripts/runCPSTest.sh - References Selenium, test_cps_personal.rb, and Quest. Don't think this is live.
  • tools/bin/scripts/runTest.sh - References Selenium and ruby_unit_tests.rb. Don't think this is live.
  • tools/bin/scripts/syncPackages.sh - It looks like it syncs code from a system-local PEAR to ./packages. Don't think its relevant in composer-era.

These might merit a little more thought:

  • tools/bin/scripts/ckeditorConfigScraper.php - This seems like something that might plausibly be used? ping @colemanw
  • tools/bin/scripts/postIPN.sh - This is tiny but interesting. It is a nice idea to track/share some of these mock calls, although the URL http://crm_32 suggests that this script specifically hasn't been "live" in a long time, and you'd need more organization/documentation to follow-through on the concept. (ED: My guess is that this style of interaction faded out as phpunit adoption improved.)

@totten totten merged commit b0a1804 into civicrm:master Jan 17, 2022
@eileenmcnaughton eileenmcnaughton deleted the tools branch January 17, 2022 22:45
@eileenmcnaughton
Copy link
Contributor Author

I definitely think the postIPN.sh should go - it is the wrong url for starters and really doesn't add anything I can see to our other ways of testing IPNs

eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jan 17, 2022
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jan 17, 2022
@eileenmcnaughton
Copy link
Contributor Author

I history on the ckeditor one suggests it is used - I put up PRs for the others

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants