Closed viniciusdacal closed 9 years ago
Nice work!
It would be nice if we moved the metadata.layout
normalization to the normalizeMetaData
function, by adding a defaultLayout
parameter to it.
Or we could think ahead a bit and add a defaults
parameter which receives an object.
Yeah, that is a great idea. @UltCombo. I made de changes, could you check?
Nice! :smile: I just did a full review, excellent work @viniciusdacal! I've added some small adjustments as well: https://github.com/JSRocksHQ/harmonic/commit/cb6a021847905b3a9456dd9ca8d37466229ed27e (please comment if you find anything out of order).
It would be nice to have an unit test for custom layouts, so I'm tagging this issue with "needs tests" so we don't end up forgetting about it.
We can publish a new Harmonic release once we update the wiki page. That wiki page seems a little outdated though, we should fully revise it. (cc @jaydson)
@viniciusdacal By the way, would you like to have Contributor status in the Harmonic repository?
Regardless of your decision, we will not push responsibilities onto you, we don't want to force you to contribute more than what you're already doing. With Contributor status, though, you will be able to help revising PRs and shaping the future of Harmonic.
So what do you think?
@UltCombo of course, it would be a pleasure to have a contributor status :smiley: Very thanks.
I've sent an invitation, GitHub should mail you shortly. Welcome aboard! :tada:
I already received here. Really thanks :smile:
Welcome @viniciusdacal, and thanks for the awesome work you're doing, man. Let's evolve this beast called Harmonic :smiling_imp:
:metal: :)
We can release a new version with this new feature.
I think the Wiki page is fine, @UltCombo. Perhaps just add the new property and document it properly.
@jaydson There are some other things that seem outdated to me. The published
metadata is now always supported independent of the theme, right? It is also missing some properties such as authorPicture
(or is this supposed to be a custom property?).
I DUNNO.
This headers thing was poorly implemented at first (my fault, again).
Looking close now, I think we should isolate the defaults and custom properties.
I mean, the title
is the post title Harmonic uses internally, but what if the user remove it? Or what if the user add a Title
property?
Well, this works just fine for now, but we can improve those headers properties a lot.
Perhaps we should have defaults
and custom
properties, or something like that.
What do you guys think?
@UltCombo @jaydson Review needed