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.

Sign up for GitHub

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

Jump to bottom

[#72] Remove unused code from catcher models #146

Merged
merged 5 commits into from
Jul 16, 2023
Merged

[#72] Remove unused code from catcher models #146

merged 5 commits into from
Jul 16, 2023

Conversation

Eclipse-Dominator
Copy link
Contributor

@Eclipse-Dominator Eclipse-Dominator commented Jun 23, 2023 •
edited
Loading

Summary:

Part of PRs for #72
Depends on #145 (Review this only after this PR is merged)

Changes Made:

  • Removes unused models
  • Removes unused functions from Issues
  • Removes unused package imports

Commit Message:

As WATcher is ported from CATcher, there are a lot of models and methods
that are not used and are unrelated to WATcher.

Let's remove most of the unused code in core/models

@Eclipse-Dominator
Remove unused code related to mutations on Github
d27b4ab
@Eclipse-Dominator
Remove package ngx-markdown
9b67047 
ngx-markdown is a markdown formatter/parser that was ported over from
CATcher. As WATcher does not use any form of markdown parser, this
package currently does not serve any use in WATcher.

Therefore, let's remove the unused package and the related components.
@Eclipse-Dominator
Removes unused components models
68ccb4a
@Eclipse-Dominator
Remove unused packages
f2bab5b
@Eclipse-Dominator Eclipse-Dominator force-pushed the 72-remove-unused-code-from-catcher-models branch from 4799495 to f2bab5b Compare June 24, 2023 05:25
@gycgabriel gycgabriel added s.Ongoing s.DoNotReview Do not review PR and removed s.Ongoing labels Jul 3, 2023
@Eclipse-Dominator Eclipse-Dominator added s.toMerge and removed s.DoNotReview Do not review PR labels Jul 15, 2023
gycgabriel
gycgabriel approved these changes Jul 15, 2023
View reviewed changes
Copy link
Collaborator

@gycgabriel gycgabriel left a comment

Choose a reason for hiding this comment

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

LGTM

@gycgabriel
Copy link
Collaborator

I noticed some of the changes are formatting, do we have husky pretty-quick working on WATcher?

gycgabriel
gycgabriel reviewed Jul 15, 2023
View reviewed changes
src/app/issues-viewer/issues-viewer.component.html
Comment on lines +2 to +3
<div class="loading-spinner" *ngIf="this.phaseService.isChangingRepo | async; else elseBlock">
<mat-progress-spinner color="primary" mode="indeterminate" diameter="50" strokeWidth="5"> </mat-progress-spinner>
Copy link
Collaborator

Choose a reason for hiding this comment

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

And this

gycgabriel
gycgabriel reviewed Jul 15, 2023
View reviewed changes
src/app/issues-viewer/card-view/card-view.component.css
Comment on lines +55 to +57
background: linear-gradient(white 33%, transparent), linear-gradient(transparent, white 66%) 0 100%,
radial-gradient(farthest-side at 50% 0, rgba(0, 0, 0, 0.5), transparent),
radial-gradient(farthest-side at 50% 100%, rgba(0, 0, 0, 0.5), transparent) 0 100%;
Copy link
Collaborator

Choose a reason for hiding this comment

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

E.g. this

@Eclipse-Dominator
Copy link
Contributor Author

Eclipse-Dominator commented Jul 16, 2023 •
edited
Loading

I noticed some of the changes are formatting, do we have husky pretty-quick working on WATcher?

We do have it working, it seems pretty-quick is reformatting those lines into those mentioned.

pretty-quick-reformat.mp4

@gycgabriel
Copy link
Collaborator

gycgabriel commented Jul 16, 2023 •
edited
Loading

Hmm, but is it doing that with each commit-push automatically? Or have to run the command manually? Odd that the dev that did the card-view-component didn't have the pretty quick done already

gycgabriel
gycgabriel approved these changes Jul 16, 2023
View reviewed changes
Copy link
Collaborator

@gycgabriel gycgabriel left a comment

Choose a reason for hiding this comment

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

LGTM

@Eclipse-Dominator Eclipse-Dominator merged commit adf0ae4 into CATcher-org:main Jul 16, 2023
joyngjr added a commit to joyngjr/WATcher that referenced this pull request Jul 16, 2023
@joyngjr
Merge commit '36a0b41c369517022086d36ea6583625f4308289' into remove-a…
a2509b1 
…ll-button

* commit '36a0b41c369517022086d36ea6583625f4308289':
  [CATcher-org#72] Remove unused code from catcher models (CATcher-org#146)
  [CATcher-org#72] Remove package ngx-markdown (CATcher-org#145)
  Extract method from header.component into phase.service.ts (CATcher-org#162)
  [CATcher-org#72] Remove code related to creating and updating repos (CATcher-org#143)
  Autofill repository URL with Browser Cache (CATcher-org#159)
  Update PR template (CATcher-org#157)
  Update Readme (CATcher-org#158)
  Enclose PR/issues in scrollable container (CATcher-org#148)
  Show loading spinner on switch repository (CATcher-org#151)
  Add wrap for username in issues-viewer's card-view (CATcher-org#147)
  Update website title after changing repository (CATcher-org#141)
  Disable milestone filter if there are no milestones (CATcher-org#149)
  Fix pressing back to cause issues to being cleared (CATcher-org#142)
joyngjr added a commit to joyngjr/WATcher that referenced this pull request Jul 18, 2023
@joyngjr
Merge commit '80f84aeb6686bb9cae9e58463baf8942fc317256' into label-wr…
6d2148d 
…apping

* commit '80f84aeb6686bb9cae9e58463baf8942fc317256':
  Add reset labels feature (CATcher-org#150)
  Allow keyboard enter when changing repository (CATcher-org#177)
  Fix ng test type error (CATcher-org#175)
  Reduce code duplication in repository url suggestions (CATcher-org#170)
  Fix hover, click interaction with issue cards (CATcher-org#173)
  [CATcher-org#72] Remove unused code from catcher models (CATcher-org#146)
  [CATcher-org#72] Remove package ngx-markdown (CATcher-org#145)
  Extract method from header.component into phase.service.ts (CATcher-org#162)
  [CATcher-org#72] Remove code related to creating and updating repos (CATcher-org#143)
  Autofill repository URL with Browser Cache (CATcher-org#159)
  Update PR template (CATcher-org#157)
  Update Readme (CATcher-org#158)
  Enclose PR/issues in scrollable container (CATcher-org#148)
  Show loading spinner on switch repository (CATcher-org#151)
  Add wrap for username in issues-viewer's card-view (CATcher-org#147)
  Update website title after changing repository (CATcher-org#141)
  Disable milestone filter if there are no milestones (CATcher-org#149)
  Fix pressing back to cause issues to being cleared (CATcher-org#142)
joyngjr added a commit to joyngjr/WATcher that referenced this pull request Jul 22, 2023
@joyngjr
Merge branch 'main' into format-assignee-component
1a69b8b 
* main:
  Add reset labels feature (CATcher-org#150)
  Allow keyboard enter when changing repository (CATcher-org#177)
  Fix ng test type error (CATcher-org#175)
  Reduce code duplication in repository url suggestions (CATcher-org#170)
  Fix hover, click interaction with issue cards (CATcher-org#173)
  [CATcher-org#72] Remove unused code from catcher models (CATcher-org#146)
  [CATcher-org#72] Remove package ngx-markdown (CATcher-org#145)
  Extract method from header.component into phase.service.ts (CATcher-org#162)
  [CATcher-org#72] Remove code related to creating and updating repos (CATcher-org#143)
  Autofill repository URL with Browser Cache (CATcher-org#159)
  Update PR template (CATcher-org#157)
  Update Readme (CATcher-org#158)
  Enclose PR/issues in scrollable container (CATcher-org#148)
  Show loading spinner on switch repository (CATcher-org#151)
  Add wrap for username in issues-viewer's card-view (CATcher-org#147)
  Update website title after changing repository (CATcher-org#141)
  Disable milestone filter if there are no milestones (CATcher-org#149)
  Fix pressing back to cause issues to being cleared (CATcher-org#142)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gycgabriel gycgabriel gycgabriel approved these changes

@chunweii chunweii Awaiting requested review from chunweii

Assignees
No one assigned
Labels
s.toMerge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Eclipse-Dominator @gycgabriel