search

stencilproject / Stencil

Stencil is a simple and powerful template language for Swift.
https://stencil.fuller.li
BSD 2-Clause "Simplified" License
2.34k stars 224 forks source link

Context param for Include block #214

Closed yonaskolb closed 6 years ago

yonaskolb commented 6 years ago

This is a redo of #95 which only includes the context param which gets passed to the file. I've also removed the using from the param

{% include "file.stencil" childContext %}
yonaskolb commented 6 years ago

@ilyapuchka @AliSoftware Could I get a review on this?

AliSoftware commented 6 years ago

@yonaskolb Also, I'm curious as to why you pushed that on your fork — which should not be necessary anymore now that you're a core contributor and have push access — instead of pushing that branch directly on the original repo?

I'm guessing it's because it's a branch that were already open in your end before you became one of the co-owners (actually you don't seem to have accepted my invitation to be part of the team?), but that would still give you some more power (add reviewers, etc), so 😉

Usually in most projects, CI is disabled for forks for security reasons (would otherwise allow some ill-intended user to expose tokens otherwise, iirc), so pushing a branch on the main repo rather than the fork is generally preferred when possible also for that reason (even if idk if that's the case for Stencil, would have to check on travis). Also would allow you to get rid of your fork and avoid having to maintain 2 different remotes for the same project and the pain of having to keep them in sync :wink:

yonaskolb commented 6 years ago

Thanks @AliSoftware, comments addressed.

Yeah if no one person is managing releases we should add changelog entries in PRs 👍

Yes, the branch is just an older one from my fork. Any future work will be in this repo 👍