-
Notifications
You must be signed in to change notification settings - Fork 31
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
Restore hash #174
Restore hash #174
Conversation
var projects = Object.keys(pkgProjects).forEach(function(project){ | ||
pathObject[project] = pkgProjects[project].hashStorage; | ||
}) | ||
return pathObject; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can do nicer with reduce
instead.
return Object.keys(pkgProjects).reduce(function(pathObject, project){
pathObject[project] = pkgProjects[project].hashStorage;
}, {});
And don't forget the ;
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And when you use the reduce
version, you don't need the extra iife/function/closure.
I commented on a few things, but really great work so far! |
@arian nice feedback! will fix during the day. |
@@ -47,4 +63,11 @@ module.exports = function(app){ | |||
app.get('/more/guides', more, guides.index); | |||
app.get('/more/guides/:guide', more, guides.article); | |||
|
|||
// hash build redirect | |||
var regex = /more\/([a-z]+[0-9]+[a-z0-9]*|[0-9]+[a-z]+[a-z0-9]*)$/; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as for core
, we can probably just redirect /more/:hash
, rather than a regex.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also makes it possible to have "custom" hashes, in case we'd want to do that for some reason. Old website functionality did have that as well, I don't know if any exist.
Updated, review welcome :) |
Awesome, reviewing... |
- now it adds the hash link to the download file, as it used to be - improved error handling in the builderHash - use a separate config file for the locations, so you can specify that in dev and production.
Fix when there are no results for loadHash
I made some improvements: SergioCrisostomo#1 |
Restore hash
Restore hash functionality in builder. (fixes #169)
Restore visual behaviour when selecting modules. (fixes #171)
Ok, this went a big pull request.
Basically it:
core/5jhj546oh54uhyfld98
tocore/builder/5jhj546oh54uhyfld98
- same for More.tests/database/
which are usable for testing this and eventually future specs. (Maybe a fallback to those files should be added for when running the site locally/in development)Thank you @timwienk for help on this