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

Remove unused plugins: copyright, scod, heartbeat, theme and mobile #2317

Closed
jdlrobson opened this issue Aug 17, 2019 · 21 comments · Fixed by #4470
Closed

Remove unused plugins: copyright, scod, heartbeat, theme and mobile #2317

jdlrobson opened this issue Aug 17, 2019 · 21 comments · Fixed by #4470
Labels
Lead: @cclauss Issues overseen by Chris (Python3 & Dev-ops lead 2019-2021) [managed] Priority: 3 Issues that we can consider at our leisure. [managed] Type: Refactor/Clean-up Issues related to reorganization/clean-up of data or code (e.g. for maintainability). [managed]

Comments

@jdlrobson
Copy link
Collaborator

Description

https://github.com/internetarchive/openlibrary/tree/master/openlibrary/plugins

While combing through the frontend I came across some plugins (and TEMPLATES) that do not seem to be enabled anywhere. Those are: copyright,heartbeat, scod, theme and mobile (there may be others but these ones i can confirm). I wasted a lot of time trying to cleanup the frontend in these files. I'd rather not waste this time ago.

These were added around 10 years ago, and have only recently seen linting improvement.
If these are not enabled in production can they be removed? It would make the codebase a lot easier to work with if we cleaned out things that are no longer active.

scod = Scan on Demand plugin
mobile = a separate mobile site. We don't have one.

Evidence / Screenshot (if possible)

When I remove these folders in the plugins folder nothing breaks. The site continues as it always has but with less lines of code.

Expectation

We should not be wasting time having to sift through files that are unused when trying to do things like revamp frontend and backend architecture.

Details

  • Logged in (Y/N)?
  • Browser type/version?
  • Operating system?

Proposal & Constraints

Stakeholders

@cclauss and @mekarpeles

@jdlrobson jdlrobson added Type: Bug Something isn't working. [managed] Priority: 1 Do this week, receiving emails, time sensitive, . [managed] Type: Refactor/Clean-up Issues related to reorganization/clean-up of data or code (e.g. for maintainability). [managed] labels Aug 17, 2019
@cclauss
Copy link
Contributor

cclauss commented Aug 17, 2019

Highly supportive. There are no bugs in code that does not exist. ;-)

@xayhewalo
Copy link
Collaborator

xayhewalo commented Oct 20, 2019

@mekarpeles @hornc I'm not sure who should be assignee to this issue, but I assumed it would be one of you two. Let me know your thoughts.

edit: typo

@xayhewalo xayhewalo added Needs: Triage This issue needs triage. The team needs to decide who should own it, what to do, by when. [managed] State: Backlogged labels Oct 20, 2019
@tfmorris
Copy link
Contributor

I don't know anything about these plugins, but it seems like if there were a PR per plugin that removed it, they could be reviewed individually and cleaned up. ie make this a series of PRs instead of an issue.

@jdlrobson jdlrobson added Patch welcomed and removed Needs: Triage This issue needs triage. The team needs to decide who should own it, what to do, by when. [managed] labels Oct 20, 2019
@xayhewalo
Copy link
Collaborator

@jdlrobson Since it sounds like you tested removing these unused plugins would you like to be assignee for this issue?

Note: being assignee doesn't necessarily mean you are responsible for doing the work. From the Managed Label Wiki:

At submission and prior to triage, the assigned owner is not necessarily the person who will fix the issue (it is not necessarily even established, at that point, if or when the issue will be fixed at all), but rather they are the person who will do as much or as little as needed to handle the issue (asking questions, soliciting input, establishing and updating the priority, checking if it is a duplicate, etc).

Once an issue is labeled State: Work In Progress, the owner is the individual doing the work, or leading/coordinating the group that is doing the work.

@jdlrobson
Copy link
Collaborator Author

It would be better to assign this to someone who knows how the Python plugins system works and that's not me.

@xayhewalo
Copy link
Collaborator

@cclauss @salman-bhai Since you are the Python3 leads, are either one of you willing to be assignee for this issue/make PRs for each plugin per @tfmorris suggestion?

@cclauss
Copy link
Contributor

cclauss commented Oct 23, 2019

Not me, sorry! We have 70 days until Python 2 EOL and the time that I can dedicate to web.py, Infogami, and Open Library will be focused on compatibility with Python 3. I am supportive of this effort but will not directly engage.

@tfmorris
Copy link
Contributor

tfmorris commented Oct 23, 2019

I'm not sure why every issue needs an assignee. We have way more issues than people to work on them all the employees are being tasked to work on things just for show, so they're not even available.

I think we should adjust the guidelines to acknowledge that there are some things which are important, but not assigned to anyone.

@xayhewalo
Copy link
Collaborator

@tfmorris

This issue isn't the best place to hold this discussion, so I'll respond to your comment on #2028

@mekarpeles mekarpeles added Priority: 2 Important, as time permits. [managed] and removed Priority: 1 Do this week, receiving emails, time sensitive, . [managed] labels Oct 27, 2019
@tfmorris
Copy link
Contributor

I don't think something can be both a refactoring and a bug. They seem mutually exclusive. This seems to clearly be a refactoring. It's also exceedingly unlikely that a refactoring, particularly dead code removal, would be considered high priority.

@xayhewalo xayhewalo removed the Type: Bug Something isn't working. [managed] label Oct 28, 2019
@xayhewalo
Copy link
Collaborator

Assigning @cdrini per slack discussions since he's doing a lot of the refactoring

@mekarpeles mekarpeles added the Lead: @cdrini Issues overseen by Drini (Staff: Team Lead & Solr, Library Explorer, i18n) [managed] label Dec 18, 2019
@mekarpeles mekarpeles added Needs: Help Issues, typically substantial ones, that need a dedicated developer to take them on. [managed] and removed Patch welcomed labels Jan 23, 2021
@mekarpeles mekarpeles added Priority: 3 Issues that we can consider at our leisure. [managed] and removed Priority: 2 Important, as time permits. [managed] labels Jan 23, 2021
@mekarpeles
Copy link
Member

Going to move to @cclauss to equalize our Lead load and lowering the priority

@mekarpeles mekarpeles added Lead: @cclauss Issues overseen by Chris (Python3 & Dev-ops lead 2019-2021) [managed] and removed Lead: @cdrini Issues overseen by Drini (Staff: Team Lead & Solr, Library Explorer, i18n) [managed] labels Jan 23, 2021
@mekarpeles
Copy link
Member

#2317 (comment)

not every issue needs an assignee, but it makes sense in our new world #4282 for every issue to have a Lead (a point capable of prioritizing, reviewing, answering questions + fielding comments on issues, finding others to help out, and helping them get setup / find the right files).

@SaravgiYash
Copy link
Contributor

@cclauss I would like to work on this issue. Any starting point?

@cclauss
Copy link
Contributor

cclauss commented Jan 23, 2021

Was this issue closed in #2318 er what?

@cclauss
Copy link
Contributor

cclauss commented Jan 23, 2021

@Yashs911 perhaps search on each of the five keywords and see if there is any cruft lying around in our codebase. If so, separate PRs for each keyword found please.

@SaravgiYash
Copy link
Contributor

SaravgiYash commented Jan 23, 2021

@Yashs911 perhaps search on each of the five keywords and see if there is any cruft lying around in our codebase. If so, separate PRs for each keyword found please.

@cclauss I just found some old comments and few references
image
image
image

@cclauss
Copy link
Contributor

cclauss commented Jan 23, 2021

Cool. Let’s keep this effort separate from #3483 because I think we can land these mods quickly whereas the “unnecessary imports” require careful review because of import side effects which flake8 can not detect.

@SaravgiYash
Copy link
Contributor

@cclauss Okay, So should I create a PR and remove all these comments and references?

@cclauss
Copy link
Contributor

cclauss commented Jan 23, 2021

One PR for each keyword (copyright, scod, heartbeat, theme, mobile) found, please.

@SaravgiYash
Copy link
Contributor

@mekarpeles mekarpeles removed the Needs: Help Issues, typically substantial ones, that need a dedicated developer to take them on. [managed] label Oct 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Lead: @cclauss Issues overseen by Chris (Python3 & Dev-ops lead 2019-2021) [managed] Priority: 3 Issues that we can consider at our leisure. [managed] Type: Refactor/Clean-up Issues related to reorganization/clean-up of data or code (e.g. for maintainability). [managed]
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants