Conversation
|
|
||
| notable changes for end users: | ||
|
|
||
| * dropped support for obsolete Node versions; now requires Node v18 or later |
There was a problem hiding this comment.
🤔 Is there a more concise (yet elegant) way to express this?
There was a problem hiding this comment.
Yes, there is:
requires Node v18 or later
There was a problem hiding this comment.
I had considered that, but wanted to be explicit about dropping support. Perhaps:
bumped Node requirement to v18 or later, dropping support for obsolete versions
🤷
There was a problem hiding this comment.
Okay, works for me 👍 (I would say that "requires version xy" in a changelog implies that this drops support for older versions.)
| notable changes for end users: | ||
|
|
||
| * dropped support for obsolete Node versions; now requires Node v18 or later | ||
| * reduced number of dependencies |
There was a problem hiding this comment.
This sounds far too mundane: It's a significant improvement IMO, but ... 🤷
There was a problem hiding this comment.
I would keep it. Most people won't care, but I think it is still worth putting four words in the changelog.
There was a problem hiding this comment.
My concern was more that this sounds like a boring line item when it's really "yay, look at what @moonglum figured out!"...
| notable changes for end users: | ||
|
|
||
| * dropped support for obsolete Node versions; now requires Node v18 or later | ||
| * reduced number of dependencies |
There was a problem hiding this comment.
Also, I opted not to list "updated dependencies" as a separate point here; seems implied?
| * switched to named instead of default exports | ||
|
|
||
| this might affect users of utility functions like `createFile` or | ||
| `resolvePath` |
There was a problem hiding this comment.
I'm not sure whether this is even worth mentioning, but it also doesn't hurt? (See de0a193 for details.)
There was a problem hiding this comment.
Will this break any of the official plugins? 😓
There was a problem hiding this comment.
I didn't think so, but it appears I'd overlooked that both faucet-static and faucet-images use FileFinder... 😬
Relatedly, I recently realized that some projects (including Node itself) use lib/internal to distinguish exports. Not ideal (and a bit enterprisey), but the only realistic alternative seems resorting to a _ prefix for such cases...
There was a problem hiding this comment.
I'm a bit scared of this change. Maybe we should skip it for a patch version 🙈
There was a problem hiding this comment.
Unfortunately, reverting that commit isn't easy.
I've got a patch that might restore backwards compatibility with stuff like this:
let exports = module.exports = function() {
console.error("WARNING: default exports are deprecated; please switch to named exports");
return exports.createFile.apply(this, arguments);
};
exports.createFile = function createFile(filepath, contents) { /* … */ }However, turns out that ae3f548 also introduced breaking changes for parseCLI, readConfig, pluginsByBucket, loadExtension, static and live by turning them async. Are we worried about that?
There was a problem hiding this comment.
instead, that might be a pragmatic solution for ... which problem again?
I remember we had some really frustrating experiences for people because as soon as npm decides to install two versions of faucet core, everything breaks. And I mean EVERYTHING 😂 This left me paranoid, especially because I can't remember what exactly causes this to happen 🙈
I'd rather have faucet-images depend on faucet-static instead
This is actually a valid point. FileFinder could move into static, and images could depend on that 🤔 images is basically a copy of static, with some processing sprinkled on top. So it would be a chance for some deduplication. This would not only be for aesthetics: There is a bug in static (faucet-pipeline/faucet-pipeline-static#24) which is also in images, as they share the same code. So if it was fixed in static, it would automatically be fixed in images as well 🤔 Is it okay for you if I try that out before we do the release? Then all packages except aiur (which is < 1) would be compatible with 2 and 3.
That release plan seems correct to me. 👍
Great! 👍 Yes, let's run npm outdated for each of the packages before we release it, and go to node 20 as the minimal version.
There was a problem hiding this comment.
Is it okay for you if I try that out before we do the release?
Sure, knock yourself out. You might want to consider #148 while you're at it - otherwise just reject that.
There was a problem hiding this comment.
I just noticed that your release plan omitted nunjucks, which also continues to support faucet-core v2.
2 participants
No description provided.