Some trivial fixes to new e2e harness #9706
Conversation
shahata
commented
Oct 20, 2014
shahata
commented
- Removing some redundant file (I hope I'm not missing anything)
- Add body tag to fixture + indentation
- Minor refactor to editting jquery script tag
- Update npm shrinkwrap
|
the missing body tag is pointless --- the rest is fine. |
|
In the future, please leave comments on a PR instead of doing this |
|
Sure, I only got around now to looking at this (after it was merged). This is some really cool stuff! |
|
I put those things in separate commits only so you'll be able to pick which of those you want, do you prefer I squash everything except for the body tag thing? |
|
no problem --- I don't really have anything against adding the body tag, so it basically LGTM --- but it just adds a bunch of gunk to the fixture that isn't really needed. We often write layout tests this way because it helps minimize the amount of code in the test files, which is really nice. Don't worry about squashing, but I'd rather the body tag commit were removed --- just to keep the fixture a bit smaller |
|
Dropped. Should I merge this then? |
|
yup, lgtm |
|
actually hang on --- @petebacondarwin can you make sure the npm-shrinkwrap is okay first? |
some packages were using versions that do not match semver@4 semantics and therefore generated errors when trying to create shrinkwrap with npm@2.x. this shrinkwrap will make it much easier to update the shrinkmap from now on
It's tinier than jsdom, and seems to work okay for generating the fixtures.
|
so, still waiting on pete to ok the changes, but I think it should be okay --- and if not, it probably won't affect anyone right away :> |
some packages were using versions that do not match semver@4 semantics and therefore generated errors when trying to create shrinkwrap with npm@2.x. this shrinkwrap will make it much easier to update the shrinkmap from now on Closes #9706
|
Sorry I missed that one. Generally I have been editing the npm-shrinkwrap.json file after cleaning to make sure it is only updating the versions of modules that I specifically want to change. In this diff it has updated readable-stream in a number of places, which could or could not have an impact. It is safer not to do this, IMO. @caitp or @shahata - can you see if |
|
@petebacondarwin I intentionally added the updates to readable-stream and sting_decoder as a separate commit since I wasn't sure you'd like that :). I think it is a good idea though since those two packages use a semver@4 incompatible version which is probably why @caitp had some problem with generating the npm shrinkwrap with npm@2. Now shrinkwrap will work without any errors. |
