-
Notifications
You must be signed in to change notification settings - Fork 25
Conversation
@@ -14,115 +14,113 @@ This table contains the status of every Polymer 3.0 element being run through au | |||
|
|||
## Support Table | |||
|
|||
| repo | `npm install` | `npm test` | reason? | | |||
| repo | *npm install* | *npm test* | reason? | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it would be good to show the command used here instead or not tie it to the word npm?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, looking at this now this was supposed to stay npm install
, wonder if I changed it on purpose or by accident.
These are the commands run within each repo in question. Would $ npm install
convey this more? Something else? Open to suggestions!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't notice that point until you mentioned it. 👍 I think it is fine with just the back ticks
I was thinking that you were using yarn install
instead of npm install
since AFAIK flat: true
in the package.json
won't do anything unless using yarn
currently.
polymer-modulizer/src/test-workspace.ts
Lines 52 to 54 in 0deae2a
// TODO(fks): Get `yarn install --flat` working to test flat install | |
// See: https://github.com/Polymer/polymer-modulizer/issues/254 | |
return exec(repo.dir, 'npm', ['install']); |
Secondly, the package.json
we are generating is not generating or inheriting a scripts
section which is where NPM looks for the command to run when npm test
is called. So with that being said, I saw that you are using wct --npm
for the test command currently.
polymer-modulizer/src/test-workspace.ts
Line 64 in 0deae2a
const {stdout, stderr} = await exec(repo.dir, 'wct', ['--npm']); |
paper-input
which can't run both browsers in parallel?
So currently I would imagine it being:
| repo | `npm install` | `wct --npm` | reason? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stramel great point, I agree | repo |
npm install|
wct --npm | reason? |
makes the most sense here, with a note about why we're testing npm install
vs. yarn install
at the moment.
@FredKSchott should the be updated as of yesterday? |
Merging this now so that we show the latest data as linked to from the blog post, can address table headers in future PR |
No description provided.