Skip to content
This repository was archived by the owner on Sep 24, 2025. It is now read-only.

Clean up code #679

Merged
merged 12 commits into from
Nov 5, 2020
Merged

Clean up code #679

merged 12 commits into from
Nov 5, 2020

Conversation

@donggyu04
Copy link
Contributor

@donggyu04 donggyu04 commented Oct 22, 2020 •
edited
Loading

Clean up

  1. Deprecated.
  2. Unused.
  3. Indentation.
  4. Redundant default parameter
  5. IED(Intellij) warning.

@donggyu04 donggyu04 self-assigned this Oct 22, 2020
@donggyu04 donggyu04 force-pushed the feature/clean-up-code branch 10 times, most recently from 314f668 to fe62578 Compare October 23, 2020 03:24
@donggyu04 donggyu04 force-pushed the feature/clean-up-code branch 2 times, most recently from 6d529b7 to c504e72 Compare October 23, 2020 03:56
junoyoon
junoyoon reviewed Oct 28, 2020
View reviewed changes
.../main/java/org/ngrinder/compatibility/perftest/controller/PerfTestFormTypeApiController.java
@RestController
@RequestMapping(value = "/perftest/api", consumes = APPLICATION_FORM_URLENCODED_VALUE)
@Deprecated
public class PerfTestFormTypeApiController extends PerfTestApiController {
Copy link
Contributor

@junoyoon junoyoon Oct 28, 2020

Choose a reason for hiding this comment

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

@RequireArgsConstructor

Copy link
Contributor Author

@donggyu04 donggyu04 Nov 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

PerfTestFormTypeApiController is a child class from PerfTestApiController.

@RequireArgsConstructor can't call the parent constructor.

ref: https://stackoverflow.com/a/29771875/8074178
related issue: projectlombok/lombok#375

ngrinder-controller/src/main/java/org/ngrinder/script/repository/FileEntryRepository.java Outdated
if (dirEntry.getKind() == SVNNodeKind.DIR) {
script.setFileType(FileType.DIR);
} else {
script.getFileType();
Copy link
Contributor

@junoyoon junoyoon Oct 28, 2020

Choose a reason for hiding this comment

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

Check if this is necessary

Copy link
Contributor Author

@donggyu04 donggyu04 Nov 4, 2020

Choose a reason for hiding this comment

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

It sets fileType if null.

https://github.com/naver/ngrinder/blob/develop/ngrinder-controller/src/main/java/org/ngrinder/script/model/FileEntry.java#L112

ngrinder-controller/src/main/java/org/ngrinder/script/repository/FileEntryRepository.java
outputStream = new ByteArrayOutputStream();
SVNProperties fileProperty = new SVNProperties();
// Get File.
repo.getFile(path, revision.getNumber(), fileProperty, outputStream);
Copy link
Contributor

@junoyoon junoyoon Oct 28, 2020

Choose a reason for hiding this comment

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

make try start here

Copy link
Contributor Author

@donggyu04 donggyu04 Nov 4, 2020

Choose a reason for hiding this comment

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

Above try block enclose SVNException, If move the block to here, we should have nested try-catch.

@donggyu04 donggyu04 force-pushed the feature/clean-up-code branch 6 times, most recently from a85df41 to 59aca99 Compare November 4, 2020 05:48
@donggyu04 donggyu04 force-pushed the feature/clean-up-code branch 2 times, most recently from a27f729 to c85f089 Compare November 4, 2020 23:40
@donggyu04 donggyu04 force-pushed the feature/clean-up-code branch 2 times, most recently from e0d3380 to ad07970 Compare November 5, 2020 00:32
imbyungjun
imbyungjun previously approved these changes Nov 5, 2020
View reviewed changes
@donggyu04 donggyu04 force-pushed the feature/clean-up-code branch from ad07970 to 8153da1 Compare November 5, 2020 02:51
@songeunwoo
Copy link
Contributor

songeunwoo commented Nov 5, 2020

Thank you for your efforts.
The code looks good :)

@songeunwoo songeunwoo merged commit 8a07f3d into develop Nov 5, 2020
@donggyu04 donggyu04 deleted the feature/clean-up-code branch November 11, 2020 01:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

3 more reviewers

@junoyoon junoyoon junoyoon left review comments

@songeunwoo songeunwoo songeunwoo approved these changes

@imbyungjun imbyungjun imbyungjun left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@donggyu04 donggyu04

Labels

3.5.3 more test required

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@donggyu04 @songeunwoo @junoyoon @imbyungjun