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 legacy trails #1221

Merged
merged 5 commits into from Feb 19, 2015
Merged

Remove legacy trails #1221

merged 5 commits into from Feb 19, 2015

Conversation

r00k
Copy link
Contributor

@r00k r00k commented Feb 18, 2015

Once upon a time, Upcase had these things called trails.

They broke down a topic into a series of steps that could be completed.
They also linked to resources (made by thoughtbot and not) that would
help you learn a topic.

We liked these trails, but the only way to indicate you'd learned
something was to click a checkbox on a given step.

Later, we created our exercise system. After creating a handful of
unrelated exercises, we decided it would be better to create groups of
exercises to be completed in a given sequence.

We decided to call this grouping and sequencing a trail and renamed the
old trail concept legacy trails.

Having two things called "trail" at the same time was confusing, and the
legacy trails were the source of much criticism ("these are mostly links
to things outside Upcase!") We didn't like the code. Users were confused
and annoyed. Bad times.

This commit removes LegacyTrails and their associated baggage.

Now, "trail" means exactly one thing.

All is ✨ and 😂.

@r00k r00k mentioned this pull request Feb 18, 2015
@r00k r00k changed the title Remove legacy trails [WIP] Remove legacy trails Feb 18, 2015
@r00k
Copy link
Contributor Author

r00k commented Feb 18, 2015

Still having a styling issue around colors.

@r00k r00k changed the title [WIP] Remove legacy trails Remove legacy trails Feb 18, 2015
@r00k
Copy link
Contributor Author

r00k commented Feb 18, 2015

Ready for review.

@@ -1,4 +1,4 @@
body.resources-index {
body.topics.topics-show {

Choose a reason for hiding this comment

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

Avoid qualifying class selectors with an element.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ldamon Can you address the comment above por favor?

@r00k r00k force-pushed the bo-remove-legacy-trails branch 2 times, most recently from cb4621c to dbb2722 Compare February 19, 2015 20:35
Once upon a time, Upcase had these things called trails.

They broke down a topic into a series of steps that could be completed.
They also linked to resources (made by thoughtbot and not) that would
help you learn a topic.

We liked these trails, but the only way to indicate you'd learned
something was to click a checkbox on a given step.

Later, we created our exercise system. After creating a handful of
unrelated exercises, we decided it would be better to create groups of
exercises to be completed in a given sequence.

We decided to call this grouping and sequencing a trail and renamed the
old trail concept legacy trails.

Having two things called "trail" at the same time was confusing, and the
legacy trails were the source of much criticism ("these are mostly links
to things outside Upcase!") We didn't like the code. Users were confused
and annoyed. Bad times.

This commit removes LegacyTrails and their associated baggage.

Now, "trail" means exactly one thing.

All is ✨ and 😂.
@@ -73,7 +73,7 @@ def included_in_current_users_plan?(licenseable)
helper_method :included_in_current_users_plan?

def topics
Topic.top
Topic.all
Copy link
Member

Choose a reason for hiding this comment

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

I'm not 100% sure, but I think this should be explorable. If not, I think explorable can be removed entirely.

This column isn't accurate and is actually unused (counts for a topic's
resources are calculated in TopicWithResources).
This is necessary because we're calling `#count` on them.

This used to work, probably accidentally, because there was a `count`
column on Topic. Now, we want to use the `#count` method on
TopicWithResource, so we need to decorate.
@r00k r00k merged commit 71b4021 into master Feb 19, 2015
@r00k r00k deleted the bo-remove-legacy-trails branch February 19, 2015 22:03
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

3 participants