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

сhange recommended file extension to .nj #742

Conversation

ArmorDarks
Copy link

Since a lot of people told that there is no real reason to keep k in extension, we can make it shorter and somehow nicer.

Less typing, less unneeded verboseness and less distraction.

Related to #691

@carljm
Copy link
Contributor

carljm commented May 2, 2016

I don't really care between .njk and .nj myself, but I think we'd need a very strong reason to change it now. I know there are already other projects (both private and OSS) that have updated their practices to follow the recommendation for .njk, and there is a cost to jerking those projects around by changing the recommendation. I don't really see a "very strong reason" for that change presented here. At the very least it should be discussed on the mailing list.

@carljm carljm closed this May 2, 2016
@ArmorDarks
Copy link
Author

It's sad to realize that decision about extension in fact has been rushed out. I've looked into Google discussion, but literally decision has been made just by few people, because not much people using Google discussions those days. And more strange that it was kind of race, who pushes PR first decides the fate of extension... I wish I could push it first, but I, as hundreds of other Nunjucks users, have been completely unaware about that discussion.

Well, I guess now we have to accept it as it is, because it's too late 'fix' anything. So you're right :)

But I had to try :)

@carljm
Copy link
Contributor

carljm commented May 2, 2016

Well, the first PR to recommend .njk (#687) was filed on Feb 20, and I didn't finally merge the recommendation until Mar 14, after a second PR (#691) almost a month later. If anyone had spoken up during that month strongly in favor of a different extension, we definitely would have revisited the discussion. So it wasn't really a "push race" at all, it was just a matter of the decision being made by the people who showed up to make it, which is the way OSS usually works.

Two different GitHub PRs plus a thread on Google Groups: that decision was made more publicly and advertised for discussion in more places than most changes to nunjucks. If someone isn't following GitHub PRs or the mailing list, for more than a month, I'm not sure how I'm supposed to get their input on a decision.

@mrhyde
Copy link

mrhyde commented May 3, 2016

GitHub PR can not in any way be considered as a public notification to anyone except project maintainer and I would say it's pointless to refer to them at all. I didn't' see that one coming at all
This is not about "the way OSS usually works", it's about doing things right. I would recommend to open a poll and make a notification to nunjucks users via npm WARN as gruntjs did. Yes, it might lead to bikeshedding but at least it will be honest for everybody.

P.S.

I know there are already other projects (both private and OSS) that have updated their practices to follow the recommendation for .njk

nope... there is like none... just try github search with .njk extension

@ArmorDarks
Copy link
Author

ArmorDarks commented May 3, 2016

@carljm Can't disagree with that, you're right. I indeed completely missed that PR and even didn't know that it was hanging there for almost a month.

Though, in same time I agree with @mrhyde. Github notifications are so flawed. I'm constantly missing new PRs in my company's repository, so I'm not surprised I've missed it in Nunjucks too. And probably some other interested folks missed it too.

I was thinking about npm warning too. This sounds like unnecessary complication, but I have a feeling that such serious decision shouldn't be made so lightly.

We still have time to take necessary actions, while later... well... it will be too late.

@carljm
Copy link
Contributor

carljm commented May 3, 2016

I did try a github code search. I searched for extension:njk extends (GH won't let you code search without a search term, I figured extends should be a common keyword in nunjucks templates). I get just over 100 results for njk, and 340 for nj. That's more, but it's not overwhelmingly more, and it's certainly not "none" for njk. Even more importantly, following links to some of those njk projects does show that many of them were started in the last few weeks, making it quite likely that they used that extension following the recommendation in the docs. I do not want to punish people for following the documented recommendation by now telling them we are changing our mind and they should rename their templates. As a compromise, I might be open to updating the docs to recommend your choice of either .nj or .njk.

Regarding past process, I just don't agree that this issue is nearly important enough to deserve harassing all nunjucks users with an npm WARN notice. There's no enforcement of anything here, no changes that break anyone's code, just a recommendation that you are free to ignore if you don't like it. Short of a WARN, the only methods I have to be in touch with nunjucks users who care about its development are GitHub and the mailing list, and both of those methods were used in this case.

@ArmorDarks
Copy link
Author

ArmorDarks commented May 3, 2016

I get just over 100 results for njk, and 340 for nj

Those stats making situation even more confusing for me. 340 old projects with settled .nj against 100 newcommers? And in result we picked up njk. We definitely had to check it before making decision about final extension.

As a compromise, I might be open to updating the docs to recommend your choice of either .nj or .njk.

I'd say this is the case when deviations aren't desirable,

@niftylettuce
Copy link
Contributor

niftylettuce commented May 16, 2016

CondolidateJS seems to NOT support njk

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.

4 participants