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 unreachable code #2947

Merged
merged 15 commits into from Jun 22, 2016
Merged

Remove unreachable code #2947

merged 15 commits into from Jun 22, 2016

Conversation

joshuaclayton
Copy link

Hi all!

I've been working on a tool to identify unused code recently, and while I was listening to the episode with @kytrinyx, she'd mentioned refactoring and needing help on the app, so, here I am!

This is a fairly large set of commits, but each discrete commit outlines removal of unused code, as well as the corresponding commit(s) that led up to it becoming unreachable.

I've attempted to make each commit both as discrete and thorough as makes sense (basically, I didn't want my removal to introduce further methods needing to be removed in other commits, creating a dependency).

Two commits include removal of Rubocop configuration - disabling long classes - that I included to keep the test suite green.

I've not been able to get lineman running locally, for whatever reason, so I'd only been running the test suite and Rubocop throughout this process.

I verified this in a browser, although there are others who might know where to be more thorough.

Please let me know if there's anything else I can do to help or explain these changes; I realize it's a fair bit to go through!

This removes the instance methods track_ids and archived_in from the
profile presenter; these methods moved into an unreachable state in the
commit 38ce25d.
This removes the twitter_link instance method (and subsequently, the
entire ExercismWeb::Presenters::Sharing class), whose usage was removed
in e3648f0 and whose test was removed in
82d117d.
This removes the following instance methods:

* days
* weeks
* months

These methods' usage were removed in
acbd567.
This removes the LanguageTrack class, whose usage was removed in
c0cba48 and
5aaf4ba.

This also removes related test harness work, including a fixture file
and helper method.
This removes the assignment presenter, corresponding test, and fixture
file only used by the test. This presenter stopped being used in
d3cf318.
This removes two unused classes related to locale and location lookup.
UnknownLocale was removed in b52ea35,
while UnknownLanguage's final usage is removed in this commit;
removals from other areas of the codebase occurred in
6ab1b69 and
cd4eaca.
This removes AppPresentersHelper, which contained two methods, both of
which were unused.

ruby_track_problems_array was introduced in
671309c but was never used in
subsequent commits; usage of all_problems_json was removed in
48962e2.
This removes SubmissionsSQL and NitpicksSQL classes, as their usage was
removed in ae9251b.
This removes an unused instance method onboarding_steps on the Guest
null object. The corresponding method on User was removed in
634f751.
This removes the Submission#older_than?, as well as the scope
Submission.older_than, which were removed in
97ca051 and
c775850, respectively.

Additionally, this removes a RuboCop setting on the same Submission
file, as RuboCop was failing due to unnecessary disabling of
Metrics/ClassLength.
This removes the active_tracks helper, as well as the corresponding
ExercismWeb::Presenters::Languages class (now unused), and corresponding
test stubs that were also unnecessary.

The most recent change removing usage of active_tracks was in
ab545eb.
This removes the unused exception InvalidAttemptError, whose usage was
removed in fe314dd.
Both helper methods within ExercismWeb::Routes::Core were removed with a
restructure of the views, where the markup was removed in
c775850.
@coveralls
Copy link

coveralls commented Jun 17, 2016

Coverage Status

Coverage increased (+0.1%) to 80.287% when pulling fbc8e20 on joshuaclayton:remove-unreachable-code into 7c2ffae on exercism:master.

@kytrinyx
Copy link
Member

Oh my goodness this is brilliant! ❤️ ❤️ ❤️ (I literally have tears in my eyes because this makes me so happy and grateful).

I will be going through this carefully over the next few days!

@sharang-d
Copy link

1500 lines removed! Good job 👍

@kytrinyx
Copy link
Member

This is so good. Thank you again for taking the time to find all of this and rip it out! ❤️

@kytrinyx kytrinyx merged commit fd8cec2 into exercism:master Jun 22, 2016
@tg-z tg-z mentioned this pull request Nov 20, 2019
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.

None yet

4 participants