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

SOLR-16870: remove now dead lines of code #1759

Merged
merged 6 commits into from
Jul 22, 2023
Merged

Conversation

epugh
Copy link
Contributor

@epugh epugh commented Jul 6, 2023

https://issues.apache.org/jira/browse/SOLR-16870

Description

Remove dead lines of code now that SolrCLI.java has a printHelp method.

Solution

Delete lines.

Tests

bats tests, but going to reach out to @Willdotwhite about verifying the Windows change!

Checklist

Please review the following and check all that apply:

  • [X ] I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
  • [ X] I have created a Jira issue and added the issue ID to my pull request title.
  • I have given Solr maintainers access to contribute to my PR branch. (optional but recommended)
  • [ x] I have developed this patch against the main branch.
  • [ x] I have run ./gradlew check.
  • [ n/a] I have added tests for my changes.
  • [ n/a] I have added documentation for the Reference Guide

@epugh epugh requested a review from risdenk July 6, 2023 19:53
@Willdotwhite
Copy link
Contributor

Not sure this is ready to go yet; either my testing is flawed (below), or we're not catching the new flags:

PS C:\Users\Will\Projects\solr\solr\packaging\build\dev\bin> .\solr.cmd -help
-help is not a valid command!
PS C:\Users\Will\Projects\solr\solr\packaging\build\dev\bin> .\solr.cmd -usage
-usage is not a valid command!
PS C:\Users\Will\Projects\solr\solr\packaging\build\dev\bin> .\solr.cmd -h    
-h is not a valid command!
PS C:\Users\Will\Projects\solr\solr\packaging\build\dev\bin> .\solr.cmd --help
--help is not a valid command!
PS C:\Users\Will\Projects\solr\solr\packaging\build\dev\bin> .\solr.cmd /?    
/? is not a valid command!

Could someone confirm/deny please: should these new lines (here):

IF "%1"=="-help" goto run_solrcli
IF "%1"=="-usage" goto run_solrcli
IF "%1"=="-h" goto run_solrcli
IF "%1"=="--help" goto run_solrcli
IF "%1"=="/?" goto run_solrcli

should target the following commands:

.\solr.cmd -help
.\solr.cmd -usage
.\solr.cmd -h
.\solr.cmd --help

(And I have absolutely no idea what /? is all about)

If so, is there a roadmap item for adding aliases for those tasks somewhere else? Unless I've massively misunderstood this task, I think the SolrCLI need a tweak to SolrCLI::main to catch the new flags (which I'll open as a PR just in case).

@Willdotwhite
Copy link
Contributor

Pull request mentioned - FYI @epugh: epugh#4

@epugh
Copy link
Contributor Author

epugh commented Jul 21, 2023

Will commit this tomorrow (Saturday) ;-).

@epugh epugh merged commit 58f1761 into apache:main Jul 22, 2023
3 of 4 checks passed
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.

3 participants