Skip to content

Conversation

@eeolivas
Copy link
Contributor

dont merge this yet just want to make sure this is what we want - and figure a way to incorporate higher-level requests for hard deletes

return false;
} else if (!id.equals(other.id))
return false;
if (isActive != other.isActive)

Choose a reason for hiding this comment

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

MINOR Replace this if-then-else statement by a single return statement. rule


public Eval() {/*empty constructor needed*/}

public Eval(Integer week, Date date, EvalType evalType, Person trainee, Batch batch,

Choose a reason for hiding this comment

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

MAJOR Constructor has 8 parameters, which is greater than 7 authorized. rule

return false;
} else if (!id.equals(other.id))
return false;
if (isActive != other.isActive)

Choose a reason for hiding this comment

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

MINOR Replace this if-then-else statement by a single return statement. rule

private Integer id;

@Column(name = "s_subject")
private String subject;

Choose a reason for hiding this comment

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

MAJOR Rename field "subject" rule

return false;
}
if (!Objects.equals(this.id, other.id)) {
if (isActive != other.isActive)

Choose a reason for hiding this comment

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

MINOR Replace this if-then-else statement by a single return statement. rule

@eeolivas
Copy link
Contributor Author

there are issues with admin delete Person. did not mean to push upstream yet

@tjkemper tjkemper changed the title Soft Delete all Domains - implemented on Person for API WIP: Soft Delete all Domains - implemented on Person for API Apr 19, 2017
@octocat-jedi
Copy link

SonarQube analysis reported 8 issues

  • MAJOR 3 major
  • MINOR 5 minor

Watch the comments in this conversation to review them.

3 extra issues

Note: The following issues were found on lines that were not modified in the pull request. Because these issues can't be reported as line comments, they are summarized here:

  1. MAJOR PersonController.java#L37: The Cyclomatic Complexity of this method "getPerson" is 13 which is greater than 10 authorized. rule
  2. MINOR EvalType.java#L4: Remove this unused import 'java.util.Objects'. rule
  3. MINOR QuestionEval.java#L6: Remove this unused import 'java.util.Objects'. rule

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.

2 participants