Skip to content

Commit

Permalink
Dropping code I don't understand
Browse files Browse the repository at this point in the history 
It's not documented either by explanation or tests so we might as well remove it.
  • Loading branch information
@fclairamb
fclairamb committed Feb 25, 2024
1 parent 4d22eac commit 5a2e9ca
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 21 deletions.
4 changes: 2 additions & 2 deletions handle_files.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -593,9 +593,9 @@ func (c *clientHandler) handleGenericHash(param string, algo HASHAlgo, isCustomM
return nil
}

args, err := advSplitN(param, ' ', 3)

This comment has been minimized.

Sign in to view

Copy link
@drakkan

drakkan Feb 25, 2024

Collaborator

@fclairamb first of all sorry for not being able to help here. I have literally no time in this period.

No test case was added for this code but I guess it should handle quoted parameters like HASH "my file.txt". Maybe unquoteSpaceSeparatedParams can already handle this case. We should have some examples from @syncplify to be sure and probably this should be a separate commit

This comment has been minimized.

Sign in to view

Copy link
@fclairamb

fclairamb Feb 25, 2024

Author Owner

Well, this is not your fault.

The issue is that @syncplify made different changes in a single PR, which is not forbidden but makes passing the automated and peer review process harder.

And the core feature he wanted to add is indeed interesting for everyone (including sftpgo IMO).

So until I can make sense of what he wanted to accomplish through this part, I prefer to drop it and keep the core feature, ie the one advertised in the PR paper.

And yes, I see the idea but I couldn't find a clear documentation of that part. So unless there tests to document what is expected, I don't see the point in adding this code in the codebase.
args := strings.SplitN(param, " ", 3)

if err != nil || len(args) < 1 {
if len(args) < 1 {
c.writeMessage(StatusSyntaxErrorParameters, fmt.Sprintf("invalid HASH parameters: %v", param))

Check warning on line 599 in handle_files.go

View check run for this annotation

Codecov / codecov/patch

handle_files.go#L599 

Added line #L599 was not covered by tests
}

Expand Down
19 changes: 0 additions & 19 deletions string_utils.go
View file

This file was deleted.

0 comments on commit 5a2e9ca

Please sign in to comment.