Adds skills folder to repo and loads markdown for each skill #302 #336

lumodon · 2017-09-27T18:10:05Z

Fixes #286
Previous PR: #302

Overview

We modified the skills digest to include 'markdownFilePath'
Changed helper function 'renderMarkdown' & associated helpers to be abstract - used for modules AND skills
Created new bin meta-helper for generating folders and initial file markdowns (not useful anymore, keeping until told otherwise)
Includes styling for skills markdown view
Modified skills view file itself
Modified skills route to use 'renderMarkdown' helper

Configuration Changes

Modifies package.json scripts section - script: test to include 'skills' folder in tests
No tests written yet

Includes bin/skillsPopulate file left-over from old specs - no longer useful but keeping until told to remove

Modified skillsPopulate script Removed accidental skills-markdown.js

…nameSkill

jaredatron · 2017-09-27T18:48:33Z

package.json

@@ -9,7 +9,7 @@
    "dev": "nodemon web-server",
    "start": "node web-server",
    "db:migrate": "knex migrate:latest",
-    "test": "mocha -G -r ./mocha.config.js $(find digest web-server database backoffice | grep .test.js) ",
+    "test": "mocha -G -r ./mocha.config.js $(find digest web-server database backoffice skills | grep .test.js) ",


i dont think we'll put any tests in there. please remove this change

jaredatron · 2017-09-27T18:50:10Z

digest/skills.js

@@ -15,6 +15,7 @@ module.exports = digest => {
      rawText,
      modules: [],
      path: `/skills/${encodeURIComponent(id)}`,
+      markdownFilePath: `/skills/${id}.md`,


i know i asked for this but i've changed my mind :)

jaredatron · 2017-09-27T18:50:24Z

modules/Grocery-Queries/README.md

@@ -1,10 +1,13 @@
 # Grocery Queries Exercise

-___Note:__ This module assumes you've already installed postgresql via homebrew. If you have not done that please do back and do the [Installfest](../Installfest) module._
+___Note:__ This module assumes you've already installed postgresql via homebrew. If you


these changes should not be in this PR. please remove

Very curious that our README somehow picked up those changes. Not even sure what or whose branch that is from. Will revert... just curiously commenting.

edit: git blame helped here. Somehow my "Fixes last minute mistakes" commit introduced this new mistake.

Very curious that our README somehow picked up those changes.
Well technically you committed these changes and worse you submitted them for reviews. This is the perfect time for you to focus on leveling up your git hygiene. Get into the habit of always using git add -p and before you open a PR always: rebase off upstream/master and consider squashing and polishing your commits. double or even triple check the diff.

jaredatron · 2017-09-27T18:51:46Z

web-server/helpers.js

@@ -133,7 +133,7 @@ module.exports = app => {
        })
    }

-    response.renderMarkdown = function(markdown){
+    response.renderMarkdown = function(markdown, viewPage='markdown', extraArgs){


i dont want to do this this way. please restore these helpers and see my comment in the routes file

add this helper response.renderMarkdownFileToHTML that does most of what renderMarkdownFile does but yields the HTML. then update renderMarkdownFile to use the renderMarkdownFileToHTML

jaredatron · 2017-09-27T18:55:26Z

web-server/routes/skills.js

@@ -31,9 +31,11 @@ module.exports = app => {
    queries.getChecksForUserAndLabels({userId, labels})
      .then(checks => {
        const checked = !!checks[skill.id]
-        response.render('skills/show', {skill, checked, title: skill.name})


revert you changes to the helpers and make a new helper called renderMarkdownFileToHTML

const markdownHTML = response.renderMarkdownFileToHTML(`/skills/${skill.id}/README.md`) response.render('skills/show', {markdownHTML, skill, checked, title: skill.name})

@deadlyicon I can't imagine doing this without being async. Are you okay with:

response.renderMarkdownFileToHTML(`/skills/${skill.id}/README.md`) .then( markdownHTML => { response.render('skills/show', {markdownHTML, skill, checked, title: skill.name}) })

of course! it has to be asyc because we read files async :)

jaredatron · 2017-09-27T18:58:45Z

web-server/views/skills/show.jade


  h1.skill-show-page-heading
    input.skill-checkbox(type="checkbox", data-label=skill.id, checked=checked)
-    span !{renderSkill(skill)}
+    span.skill-markdown !{content}


this span is inside of the h1, outdent it 1 level

also make it a div by removing span

jaredatron · 2017-09-27T19:00:31Z

web-server/views/skills/show.jade

+    li
+      a(href=sourceUrl target="_blank") View On GitHub
+    li
+      a(href=editeUrl target="_blank") Improve This Page

  h1.skill-show-page-heading


remove this h1 with the checkbox. We're going to need to write some client javascript to dynamically add this checkbox on page load since we cannot easily modify the HTML we get from the markdown file. Ignore that for now and just remove this h1 and input

jaredatron · 2017-09-27T19:01:06Z

please rebase this branch off upstream/master

lumodon mentioned this pull request Sep 27, 2017

add /skills folder to the repo and load that markdown for each skill #286

Open

lumodon and others added 11 commits September 27, 2017 11:19

Modifies helpers to prepare for skill markdowns

ea5218e

Includes bin/skillsPopulate file left-over from old specs - no longer useful but keeping until told to remove

Modified skills related files(digest, views, and route)

ebc6b85

Modified skillsPopulate script Removed accidental skills-markdown.js

Added styling for individual pages

e2b7403

Abstracts original function to DRY up our changes

fd0babb

Updates usage to be more explicit

a5917bf

Modifies rename-skills function to also change skill markdowns

f247fe5

Fixes last minute mistakes

fdb3b90

Use util for converting raw text to name

c4967af

Rename renameSkillsWithinContainer to renameSkills

0f2d1ab

Merge renameSkillsWithinMarkdown and renameSkillsWithinModule into re…

327d40d

…nameSkill

Add whitespace formatting to skill markdown files

3dd21ef

EmmaEm force-pushed the 286 branch from f815392 to 3dd21ef Compare September 27, 2017 18:20

jaredatron suggested changes Sep 27, 2017

View reviewed changes

jaredatron closed this Sep 27, 2017

EmmaEm mentioned this pull request Sep 28, 2017

286 - Adds skills folder to repo and loads markdown for each skill #349

Closed

lumodon mentioned this pull request Oct 2, 2017

Impements skills markdown into skills page #363

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds skills folder to repo and loads markdown for each skill #302 #336

Adds skills folder to repo and loads markdown for each skill #302 #336

lumodon commented Sep 27, 2017

jaredatron Sep 27, 2017

jaredatron Sep 27, 2017

jaredatron Sep 27, 2017

lumodon Sep 27, 2017 •

edited

Loading

jaredatron Sep 29, 2017

jaredatron Sep 27, 2017

jaredatron Sep 27, 2017

jaredatron Sep 27, 2017

lumodon Sep 28, 2017 •

edited

Loading

jaredatron Sep 29, 2017

jaredatron Sep 27, 2017

jaredatron Sep 27, 2017

jaredatron Sep 27, 2017

jaredatron commented Sep 27, 2017

Adds skills folder to repo and loads markdown for each skill #302 #336

Adds skills folder to repo and loads markdown for each skill #302 #336

Conversation

lumodon commented Sep 27, 2017

Overview

Configuration Changes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lumodon Sep 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lumodon Sep 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaredatron commented Sep 27, 2017

lumodon Sep 27, 2017 •

edited

Loading

lumodon Sep 28, 2017 •

edited

Loading