Maintenance #74
Maintenance #74
Conversation
See #73 The main problem was the `.cjs` extensions, which confused content delivery networks. Simply changing the extension to `.js` won't work, because that'd confuse Node 12 and up. So besides changing the extension, the file has also been moved into a sub-directory containing a package.json file that tells newer Node versions that the files here are CommonJS (without the need for a file extension). This commit also adds documentation for the different ways that the module can now be obtained.
Codecov Report
@@ Coverage Diff @@
## master #74 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 1 1
Lines 110 130 +20
=========================================
+ Hits 110 130 +20
Continue to review full report at Codecov.
|
scripts/prepublish
Outdated
| sed --in-place "s/[email protected]/$pkg@$VERSION/" "$files" | ||
| sed --in-place "s/$pkg@$PREVIOUS_VERSION/$pkg@$VERSION/" "$files" |
There was a problem hiding this comment.
The --in-place flag is not supported on macOS; one must use -i '' on that platform. It may not be possible to write a portable script that uses this sed feature. Portability may not be important.
Strictly speaking the first pattern should use 0[.]0[.]0 rather than 0.0.0.
"$files" only works correctly because .config specifies only a single source file. I suggest using "${files[@]}" in conjunction with files=($(get source-files)).
There was a problem hiding this comment.
Thanks, David. The portability problem I'm not going to solve now, but I noted it down in fluture-js/fluture-template#10
The other suggestions are great and I'll add them upstream before pulling them down and updating this PR.
There was a problem hiding this comment.
Strictly speaking the first pattern should use
0[.]0[.]0rather than0.0.0.
Hm, I guess the same is true for $PREVIOUS_VERSION, which also contains periods. I'd have to regex-escape that to be truly robust, or tell sed somehow that we want a literal match.
LICENSE
Outdated
| Copyright (c) 2019 Aldwin Vlasblom | ||
| Copyright (c) 2021 Aldwin Vlasblom |
There was a problem hiding this comment.
Does this not happen automatically during the release process?
There was a problem hiding this comment.
It does, but this is an upstream (from the template repo) change that got included automatically upon merging. So let's say it gets updated in one of two ways. :)
| sed --in-place "s/[email protected]/$pkg@$VERSION/" "${files[@]}" | ||
| sed --in-place "s/$pkg@$PREVIOUS_VERSION/$pkg@$VERSION/" "${files[@]}" |
There was a problem hiding this comment.
Did you consider combining the patterns?
| sed --in-place "s/[email protected]/$pkg@$VERSION/" "${files[@]}" | |
| sed --in-place "s/$pkg@$PREVIOUS_VERSION/$pkg@$VERSION/" "${files[@]}" | |
| sed -E --in-place "s/$pkg@(0.0.0|$PREVIOUS_VERSION)/$pkg@$VERSION/" "${files[@]}" |
Should substitutions be performed globally? Currently only the first occurrence of the pattern on each line will be replaced.
There was a problem hiding this comment.
Did you consider combining the patterns?
I tried and failed. I guess the -E is needed for groupings.
Should substitutions be performed globally?
Hm, that does sounds like it would be better, although now it's just being used to update URLs, each on their own line like //. [foo]: foo.example.com/[email protected].
There was a problem hiding this comment.
I'll look into this later: fluture-js/fluture-template#11
Closes #73