NikolayRys / Likely

The social sharing buttons that aren’t shabby
ISC License
395 stars 61 forks source link

Minified files in archive #178

Closed vitkarpov closed 4 years ago

vitkarpov commented 4 years ago

It turns out there are no minified files inside distributives.

NikolayRys commented 4 years ago

Great observation. But there is a little bit more to it. In agreement with the Ilya Birman, the packaged version needs to be just two files: likely.css and likely.js. So we need to amend the script itself to take only the two minified files and rename them from (likely.min.css -> likely.css).

vitkarpov commented 4 years ago

Makes sense, will do!

vitkarpov commented 4 years ago

I'd suggest change the build in a way that likely. would be minified. Instead of having .min files, we'd have *.dev files unmodified. @NikolayRys what do you think?

NikolayRys commented 4 years ago

I have considered it but this feels a bit unusual - people won't be expecting it, and immediately be able to tell what's going on, without manually open the build files. Because ".dev" kinda implies more than "min" does. It's not unreasonable to assume that "dev", for example, may have more console logging or some other instrumentation.


but maybe I have a skewed perspective as a back-end developer :)

vitkarpov commented 4 years ago

I have considered it but this feels a bit unusual - people won't be expecting it, and immediately be able to tell what's going on, without manually open the build files. Because ".dev" kinda implies more than "min" does. It's not unreasonable to assume that "dev", for example, may have more console logging or some other instrumentation.

but maybe I have a skewed perspective as a back-end developer :)

Not at all, actually I have the same feeling. From the other perspective changing the meaning here feels like backward compatibility, without explicit renaming them before zipping in. Anyway, just wanted to talk about it 😄

NikolayRys commented 4 years ago

Not sure this will work 🤔 mv release/likely.min.js release/likely.js will probably fail, because this file already exists. Have you tried it in your console?

vitkarpov commented 4 years ago

Yep, I checked it works. Probably you're confusing with moving folders when it says "directory not empty".

NikolayRys commented 4 years ago

Sorry, but I'm not yet sure it's the way to do it. Consider this: It deletes and rewrites the files in the current release. We'll need to run git reset each time after such packing. I suggest to do something like this:

mkdir release/zip
cp release/likely.min.css release/zip/likely.css
cp release/likely.min.js release/zip/likely.js
cross-env-shell bestzip release/$npm_package_name-$npm_package_version.zip release/zip/likely.css release/zip/likely.js
rm -rf release/zip
vitkarpov commented 4 years ago

@NikolayRys Excuse me for the delay, fixed! Pls, check it out once again, thanks.