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

Jump to bottom

Include "Help make this better" text on track topic pages #3475

Merged
merged 11 commits into from May 17, 2017
Merged

Include "Help make this better" text on track topic pages #3475

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
5d9789e
Include "Help make this better" text on track topic pages
Apr 22, 2017
c1ebad9
Remove unuses X::Docs::Track class
Apr 22, 2017
dea1dc1
Use "refute" instead of "assert !"
Apr 22, 2017
9b714d6
Clarify method name
Apr 22, 2017
1a44a1e
Refactor Presenters::Track#docs
Apr 22, 2017
827a210
Don't stub out the object under test
Apr 23, 2017
3afedaa
Clean up Presenters::Track tests
Apr 23, 2017
d690e72
Fix broken link on fallback content page
Apr 28, 2017
33cdb68
Check for template existance and read template in same method
Apr 29, 2017
2b34498
Explicitly name file outputs for clarity
Apr 29, 2017
483e17f
Define location and file format for shared track topic templates in s…
Apr 29, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 6 additions & 4 deletions app/presenters/track.rb
View file
Open in desktop
Expand Up @@ -47,13 +47,15 @@ def topic_content(topic_key)
end

def contributing_instructions(topic_key)
populate_template_with_trackler_data(File.read("./x/docs/md/track/BETTER.md")).gsub('TOPIC', topic_key.to_s.upcase)
template = File.read("./x/docs/md/track/BETTER.md")
populate_template_with_trackler_data(template).gsub('TOPIC', topic_key.to_s.upcase)
end

def fallback_topic_content(topic_key)
Copy link

@Insti Insti Apr 29, 2017 •
edited

Choose a reason for hiding this comment

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

The signatures of these 2 methods look very similar.

def contributing_instructions(topic_key)
and
def fallback_topic_content(topic_key)

I wonder if keyword arguments could help us here by making explicit what topic_key is going to be used for.

def contributing_instructions(replacement_value: topic_key)
and
def fallback_topic_content(file_basename: topic_key)

What do you think?

Copy link
Author

Choose a reason for hiding this comment

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

I'm a little torn. Do you think it's important that whatever's calling these methods knows how topic_key will be used for?

Here's the place where we're calling these methods now:

def topic_content(topic_key)
  if trackler_topic_content(topic_key).present?
    [trackler_topic_content(topic_key).strip, contributing_instructions(topic_key)].join("\n")
  else
    fallback_topic_content(topic_key)
  end
end

I think it's nice that topic_content doesn't need to know the implementation of contributing_instructions or fallback_topic_content. Instead, it passes along the topic_key and it gets back the relevant content.

Copy link

@Insti Insti May 1, 2017 •
edited

Choose a reason for hiding this comment

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

You make a good point, I think I'd become so focused on those two methods that I lost track of the context.

filepath = "./x/docs/md/track/#{topic_key.upcase}.md"
return '' unless File.exist?(filepath)
populate_template_with_trackler_data(File.read(filepath))
template_filepath = "./x/docs/md/track/#{topic_key.upcase}.md"
return '' unless File.exist?(template_filepath)
template = File.read(template_filepath)
Copy link

Choose a reason for hiding this comment

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

I like this 👍

populate_template_with_trackler_data(template)
end

def populate_template_with_trackler_data(template)
Expand Down