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 code related to creating and updating repos #143

Merged
merged 4 commits into from
Jul 12, 2023
Merged

[#72] Remove code related to creating and updating repos #143

merged 4 commits into from
Jul 12, 2023

Conversation

Eclipse-Dominator
Copy link
Contributor

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

Summary:

fixes part related to creating/updating repos in #72

Changes Made:

  • Removes code related to creating and modifying content on GitHub
  • Removes unused filters

Commit Message:

As a majority of the WATcher code base is ported over from CATcher,
it includes a lot of unused code. One such area is the code related to 
creating and updating issues on GitHub. Since WATcher is mainly used
to read data from GitHub and not modify data for now. 

Let's remove code that is related to modifying or creating repos

@Eclipse-Dominator Eclipse-Dominator force-pushed the 72-remove-unused-code-from-catcher-github-issue-service branch from 5e769d3 to d27b4ab Compare June 23, 2023 19:31
@Eclipse-Dominator Eclipse-Dominator mentioned this pull request Jun 23, 2023
Closed
@Eclipse-Dominator Eclipse-Dominator changed the title Remove unused code related to mutations on Github [#72] Remove unused code related to mutations on Github Jun 23, 2023
@Eclipse-Dominator Eclipse-Dominator changed the title [#72] Remove unused code related to mutations on Github [#72] Remove code related to creating and updating repos Jun 23, 2023
@Eclipse-Dominator Eclipse-Dominator mentioned this pull request Jun 23, 2023
Merged
chunweii
chunweii requested changes Jun 25, 2023
View reviewed changes
Copy link
Contributor

@chunweii chunweii left a comment

Choose a reason for hiding this comment

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

In github.service.ts, the following variables/functions are not in use:

  • MOD_ORG, DATA_REPO, fetchSettingsFile()
  • GithubService.: storeOrganizationDetails(), createRepository(), areUsersAssignable(), closeIssue(), updateIssue(), fetchDataFile() // linked to data.service.ts, fetchSettingsFile()

Since this is a rather large change, it will be good to have another developer to review this

gycgabriel
gycgabriel approved these changes Jul 3, 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 chunwei's comment, you can decide if it's in the scope of your PR and update. Otherwise I think its lgtm

@Eclipse-Dominator
Copy link
Contributor Author

I noticed chunwei's comment, you can decide if it's in the scope of your PR and update. Otherwise I think its lgtm

I have removed createRepository(), updateIssue(), closeIssue() as these functions relates more towards modifying repository which aligns with the goal of this PR.

gycgabriel
gycgabriel approved these changes Jul 3, 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

@chunweii require your approval to pass

chunweii
chunweii approved these changes Jul 11, 2023
View reviewed changes
Copy link
Contributor

@chunweii chunweii 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 5b0a469 into CATcher-org:main Jul 12, 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 chunweii approved these changes

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

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