WIP - remove dist from git #299
Conversation
|
Can you add some |
|
Eternally annoyed that browserify blows up when writing to a nonexistent directory instead of creating the necessary directories :/ Made |
|
Still testing yarn's compilation of git deps. Apparently it is quite broken: |
|
Tests are failing because current "example" tests are broken (they're broken on master, too). Opening separate issue. |
|
@searls I can get this cleaned up and ready to go if we're okay with one scenario not working: yarn currently doesn't run the npm lifecycle scripts when installing deps from github. It does when installing plain git deps, but yarn doesn't currently run the same install steps for github as they do for git. yarnpkg/yarn#5235 So... if this PR were to be merged, the only people to be impacted would be yarn users installing from github. (yarn users installing from registry; and npm users installing from github would work as intended) Are we okay with this? Personally, I am (unsurprisingly) since this is a yarn bug and I don't really feel compelled to have all the added annoyance of dealing with committing built assets to account for a bug. |
|
Well, also users who download scripts from Github and then throw them into a vendor/js directory
… On Jan 20, 2018, at 11:34, Jason Karns ***@***.***> wrote:
@searls I can get this cleaned up and ready to go if we're okay with one scenario not working:
yarn currently doesn't run the npm lifecycle scripts when installing deps from github. It does when installing plain git deps, but yarn doesn't currently run the same install steps for github as they do for git.
yarnpkg/yarn#5235
yarnpkg/rfcs#79
So... if this PR were to be merged, the only people to be impacted would be yarn users installing from github. (yarn users installing from registry; and npm users installing from github would work as intended)
Are we okay with this? Personally, I am (unsurprisingly) since this is a yarn bug and I don't really feel compelled to have all the added annoyance of dealing with committing built assets to account for a bug.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
We can just link to npmcdn for them.
On Sat, Jan 20, 2018 at 12:04 PM Justin Searls <notifications@github.com>
wrote:
… Well, also users who download scripts from Github and then throw them into
a vendor/js directory
> On Jan 20, 2018, at 11:34, Jason Karns ***@***.***> wrote:
>
> @searls I can get this cleaned up and ready to go if we're okay with one
scenario not working:
>
> yarn currently doesn't run the npm lifecycle scripts when installing
deps from github. It does when installing plain git deps, but yarn doesn't
currently run the same install steps for github as they do for git.
>
> yarnpkg/yarn#5235
> yarnpkg/rfcs#79
>
> So... if this PR were to be merged, the only people to be impacted would
be yarn users installing from github. (yarn users installing from registry;
and npm users installing from github would work as intended)
>
> Are we okay with this? Personally, I am (unsurprisingly) since this is a
yarn bug and I don't really feel compelled to have all the added annoyance
of dealing with committing built assets to account for a bug.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.
>
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#299 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAHUpEOYpqBhDfDkIjdcm6c01L82i3R6ks5tMhyEgaJpZM4Qaibv>
.
|
|
Will merge if we can:
|
In order for the package to be installable via git (and to remove dist from git), then the `prepare` script will need to be runnable from the user's machine. Which means `yarn` needs to be available to npm users.
Whitelist files rather than dealing with npmignore override of gitignore. Package now includes src, lib, dist, and TS typings. (package.json, readme, changelog, license are included automatically by npm) Things no longer included in package: regression, test, examples, docs
Browserify won't create necessary directories. So they need to be created prior to running the builds. Which also means we need to use a cross-platform `mkdir` command. And we need to run the clean command as part of build/preparation so we need cross-platform rm-rf.
Unsure how best to "advertise" the dist download. Custom badge for now. Link points to dist/ dir at latest published version.
|
I swear to god I'm not trolling you but before we merge install directions also need to be updated. What would you have it say now, since most people are using npm/webpack for building browser apps whereas we were still living in bower/curl land when we started this thing? https://github.com/testdouble/testdouble.js/blob/master/docs/1-installation.md#for-use-in-browsers |
|
I can update those. I didn't even think to check the installation docs :(
Do we intend to drop bower support entirely or just undocument it? I can
look into what bower does with this setup.
…On Mon, Feb 5, 2018 at 10:50 AM, Justin Searls ***@***.***> wrote:
I swear to god I'm not trolling you but before we merge install directions
also need to be updated. What would you have it say now, since most people
are using npm/webpack for building browser apps whereas we were still
living in bower/curl land when we started this thing?
https://github.com/testdouble/testdouble.js/blob/master/
docs/1-installation.md#for-use-in-browsers
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#299 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAHUpNiZY9WX9IkEJtIoWBmJ_i3mnJH_ks5tRyNYgaJpZM4Qaibv>
.
|
closes #290
npm runs
preparewhen a package is installed via git, so we just run compilation in the prepare script (which is incidentally the preferred script over prepublish)