JSRocksHQ / harmonic

The next static site generator
http://harmonicjs.com/
MIT License
282 stars 26 forks source link

Remove duplicated parse logic #168

Closed viniciusdacal closed 9 years ago

viniciusdacal commented 9 years ago

In pull request #167, @UltCombo mentioned about refactoring generatePosts and generatePages. I started with some thoughts here, but I need your opinions @UltCombo and @jaydson .

UltCombo commented 9 years ago

Nice work! This is definitely a step forward. Please check the comments I've added. :smile:

viniciusdacal commented 9 years ago

thanks for the suggestions :smile: I've added them in the last update. Do you see more something we could abstract here?

UltCombo commented 9 years ago

I'll take a deeper look tonight. :smile: By the way, nice work writing those helper functions. Those pure functions with single responsibility are very useful and re-usable. We can keep refactoring in this direction.

jaydson commented 9 years ago

Awesome job @viniciusdacal ! Thanks for that.

UltCombo commented 9 years ago

Merged! Fantastic work.

As for the next refactoring step, perhaps we could try to go ahead and merge both generatePosts and generatePages into a single method, and add logic branching when necessary. Here's a rough draft:

/**
 * Generate a `post` or `page` file depending on the `type` parameter.
 * @param {'post'|'page'} type - the type of the file to the generate.
 * @param {string} permalink - the file's permalink pattern taken from the Harmonic instance's config object.
 */
async generateFile(type, permalink) {
    // some common logic

    if (type === 'post') {
        // crop content, add year/month permalink replacement patterns,
        // skip if metadata.date > Date.now()
    }

    // more common logic
    // write file, push metadata to the result array, etc.
}

Not sure if this is viable though, but worth exploring.

UltCombo commented 9 years ago

I've done some extra refactoring to make those methods shorter and more alike as well: https://github.com/JSRocksHQ/harmonic/commit/5343a2fb448ddf2c21cb2278db3113985be8345a

jaydson commented 9 years ago