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

Restore hash #174

Merged
merged 11 commits into from
Nov 11, 2014
Merged

Restore hash #174

merged 11 commits into from
Nov 11, 2014

Conversation

SergioCrisostomo
Copy link
Member

Restore hash functionality in builder. (fixes #169)
Restore visual behaviour when selecting modules. (fixes #171)

Ok, this went a big pull request.

Basically it:

  • redirects core/5jhj546oh54uhyfld98 to core/builder/5jhj546oh54uhyfld98 - same for More.
  • restores the colour difference between chosen module and dependency
  • adds back the hash save/load functionality
  • adds some files under 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)
  • adds a link with the new hash (when the checkbox asking for it is checked)

Thank you @timwienk for help on this

var projects = Object.keys(pkgProjects).forEach(function(project){
pathObject[project] = pkgProjects[project].hashStorage;
})
return pathObject;
Copy link
Member

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 ;.

Copy link
Member

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.

@arian
Copy link
Member

arian commented Nov 9, 2014

I commented on a few things, but really great work so far!

@SergioCrisostomo
Copy link
Member Author

@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]*)$/;
Copy link
Member

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.

Copy link
Member

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.

@SergioCrisostomo
Copy link
Member Author

Updated, review welcome :)

@arian
Copy link
Member

arian commented Nov 11, 2014

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.
@arian
Copy link
Member

arian commented Nov 11, 2014

I made some improvements: SergioCrisostomo#1

@arian arian modified the milestone: 1.1 Nov 11, 2014
arian pushed a commit that referenced this pull request Nov 11, 2014
@arian arian merged commit 552b29b into mootools:master Nov 11, 2014
@SergioCrisostomo SergioCrisostomo deleted the restore-hash branch January 14, 2016 10:56
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.

Hashes in builder Dependencies in builder
3 participants