Remove "Could not start watchman" warning #8
Conversation
chancancode
force-pushed
the
rm-warning
branch
from
780f5a9
to
9a90a41
Compare
|
Let's remove watchman support and the warning. |
I don't think this makes sense. Removing the warning is quite different from removing support for watchman all together. |
|
I'm 👍 on the general goal here, the tests need some tweaks (and I think we'll need to do something about Node 4 in CI). |
chancancode
force-pushed
the
rm-warning
branch
3 times, most recently
from
8d15e5d
to
6f25039
Compare
|
Couldn't get yarn to work on Node 4, tried a few things including installing a yarn 1.3.2 (maybe the current-gen installer doesn't work on Node 4 or something, dunno). Since the changes in this PR are probably breaking changes for the purpose of this package anyway, I propose that we just bump major and drop Node 4 support with this. |
chancancode
force-pushed
the
rm-warning
branch
2 times, most recently
from
54c0caa
to
e0be410
Compare
Sounds good to me, we should also drop Node 6 (and update |
chancancode
force-pushed
the
rm-warning
branch
5 times, most recently
from
12c2b78
to
e4a900d
Compare
Historically, `watchman` was much more important with the use of in-project `tmp` directory. With that approach, we had a lot more fsevent to filter through. Now, with the move to using the system's TMPDIR, this problem does not really exist anymore. On the other hand, since the user most likely did not have `watchman` already installed on their computer, this "warning" is emitted to every new Ember user when they start the server. Since it sounded like something is broken, they would then have to spend time understanding the issue (following the link) and installing the package, which is quite involved on anything other than on OS X, all for (at this point, imo) dubious benefits. I propose that we remove this warning entirely. If we want to, in Ember CLI, we could potentially count the fsevents or profile the filtering code, then emit the warning only when we it becomes an issue. (Probably not worth it though?) The trigger for this was ember-learn/guides-source#1038, which was caused by ember-learn/super-rentals-tutorial#21. Since we are now capturing the real output for the tutorial, it also captured the warning (the omission previously was basically a "bug"). It felt like _something_ need to be said about it in the tutorial text, since it looks like something went wrong. The options are: 1. Pre-install `watchman` on the build server, so the warning won't show up 2. Teach the readers to install `watchman` 3. Instruct the readers to ignore the warning (1) feels like cheating without also doing (2). (2) is tricky on anything other than OS X (i.e. Linux). (3) is probably the best option, but that begs the question, why did we have the warning in the first place then? After a brief discussion on Discord, it seems like maybe the time for which this warning is necessary has past, and even core team members routinely use Ember without `watchman` installed, so removing it seemed appropriate. * * * Since the `watchmanSupportsPlatform` option is not used by anymore, I have removed it. I would think that it would be used to skip certain checks, but it is not. If that was a mistake I can bring it back.
chancancode
force-pushed
the
rm-warning
branch
from
e4a900d
to
e369461
Compare
|
Just had another interesting error message that wasn't parsed which was this was related to the permissions of |
|
@chrmod yes please, it should be a “breaking change” release which would mean a 0.2.0 I believe. |
Historically,
watchmanwas much more important with the use of in-projecttmpdirectory. With that approach, we had a lot more fsevent to filter through.Now, with the move to using the system's TMPDIR, this problem does not really exist anymore.
On the other hand, since the user most likely did not have
watchmanalready installed on their computer, this "warning" is emitted to every new Ember user when they start the server.Since it sounded like something is broken, they would then have to spend time understanding the issue (following the link) and installing the package, which is quite involved on anything other than on OS X, all for (at this point, imo) dubious benefits.
I propose that we remove this warning entirely. If we want to, in Ember CLI, we could potentially count the fsevents or profile the filtering code, then emit the warning only when we it becomes an issue. (Probably not worth it though?)
The trigger for this was ember-learn/guides-source#1038, which was caused by ember-learn/super-rentals-tutorial#21.
Since we are now capturing the real output for the tutorial, it also captured the warning (the omission previously was basically a "bug"). It felt like something need to be said about it in the tutorial text, since it looks like something went wrong.
The options are:
watchmanon the build server, so the warning won't show upwatchman(1) feels like cheating without also doing (2).
(2) is tricky on anything other than OS X (i.e. Linux).
(3) is probably the best option, but that begs the question, why did we have
the warning in the first place then?
After a brief discussion on Discord, it seems like maybe the time for which this warning is necessary has past, and even core team members routinely use Ember without
watchmaninstalled, so removing it seemed appropriate.Since the
watchmanSupportsPlatformoption is not used by anymore, I have removed it. I would think that it would be used to skip certain checks, but it is not. If that was a mistake I can bring it back.