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

IssueRespondedComponent: Decompose logical expressions into methods #599

Merged

Conversation

dingyuchen
Copy link
Contributor

@dingyuchen dingyuchen commented Feb 17, 2021

Addresses a task in #553

Changes:

  • Logical expressions in the IssueRespondedComponent for initialising default filter has been decomposed into separate methods
  • Tests have been added to check if the default filter is being initialised correctly

Proposed Commit Message:

IssueRespondedComponent: Decompose logical expressions into methods

Logic expression for initialising default filter for
IssueRespondedComponent is too long and has no tests. 

Let's
* decompose the logic expression to improve readability 
* add tests for default filter initialisation

@dingyuchen dingyuchen requested a review from a team February 17, 2021 09:28
Copy link
Contributor

@seanlowjk seanlowjk left a comment

Choose a reason for hiding this comment

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

Just two comments wrt to the tests!

Other than that, good job! :D

@codecov-io
Copy link

codecov-io commented Feb 17, 2021

Codecov Report

Merging #599 (0e74421) into master (a3959c5) will increase coverage by 0.18%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #599      +/-   ##
==========================================
+ Coverage   67.02%   67.20%   +0.18%     
==========================================
  Files          70       71       +1     
  Lines        2135     2150      +15     
  Branches      199      199              
==========================================
+ Hits         1431     1445      +14     
- Misses        663      664       +1     
  Partials       41       41              
Impacted Files Coverage Δ
...ponse/issue-responded/issue-responded.component.ts 93.33% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a3959c5...0e74421. Read the comment docs.

Copy link
Contributor

@seanlowjk seanlowjk left a comment

Choose a reason for hiding this comment

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

Thank you for the tests and refactoring! Good work! 💯

Copy link
Contributor

@ptvrajsk ptvrajsk left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Nice Job

Copy link
Contributor

@kkangs0226 kkangs0226 left a comment

Choose a reason for hiding this comment

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

Other than the comments I've made, LGTM!👍

Copy link
Contributor

@anubh-v anubh-v left a comment

Choose a reason for hiding this comment

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

Looks good, but needs some tidying up for the test descriptions.

@anubh-v
Copy link
Contributor

anubh-v commented Feb 26, 2021

@dingyuchen can you make the changes? Thanks!

@anubh-v anubh-v changed the title Default filters: Decompose logical expressions into methods IssueRespondedComponent: Decompose logical expressions into methods Feb 28, 2021
@anubh-v anubh-v merged commit faef3e4 into CATcher-org:master Feb 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants