Closed VIPv6 closed 5 years ago
Hey,
Thanks a lot for the PR! This will surely make it easier for yarn users to use this extension. Just one comment: I think many people still use npm, so could you please change the PR to include both? So it'd say for example:
"You can either use yarn
or npm
to build this extension and instructions for both are included below"
and then
npm install
or
yarn install
for each section? Take a look at how other JS projects do it for reference...
Thanks again.
Actually, Tom. the npm commands didn’t work at all for me. Some research on the package.json and the web pack.config.js led me to yarn, which worked. I think that only the yarn commands are relevant the way it currently stands.
Kevin
On Jun 5, 2019, at 12:24 AM, Tom Hacohen notifications@github.com wrote:
Hey,
Thanks a lot for the PR! This will surely make it easier for yarn users to use this extension. Just one comment: I think many people still use npm, so could you please change the PR to include both? So it'd say for example:
"You can either use yarn or npm to build this extension and instructions for both are included below"
and then
npm install or yarn install
for each section? Take a look at how other JS projects do it for reference...
Thanks again.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/tasn/webext-signed-pages/pull/21?email_source=notifications&email_token=ACSZARWNPS32PZD5W4K43RDPY5S2ZA5CNFSM4HS7OES2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODW62XZQ#issuecomment-498969574, or mute the thread https://github.com/notifications/unsubscribe-auth/ACSZARX2VMXRF7OFM25YQ4LPY5S2ZANCNFSM4HS7OESQ.
It's because it's meant to be npm run-script build
and npm run-script package
. Could you fix that as well and show the alternatives? If not, I'm happy to do that, but you'll have to rebase this patch over my changes.
Feel free to make the changes you like and I’ll rebase and do another pull request. That should be cleaner.
On Jun 5, 2019, at 10:28 AM, Tom Hacohen notifications@github.com wrote:
It's because it's meant to be npm run-script build and npm run-script package. Could you fix that as well and show the alternatives? If not, I'm happy to do that, but you'll have to rebase this patch over my changes.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/tasn/webext-signed-pages/pull/21?email_source=notifications&email_token=ACSZARVXGWPJN3ZOG7EDXR3PY7ZTBA5CNFSM4HS7OES2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXAN6QY#issuecomment-499179331, or mute the thread https://github.com/notifications/unsubscribe-auth/ACSZARQJLJ4G54ZQWCNEM6LPY7ZTBANCNFSM4HS7OESQ.
Done, thanks!
I’m sorry, Tom. I can’t figure out how to do a rebase. I found the button in SourceTree, but I asks me to select a commit and doesn’t allow me to select a commit. I can’t find the rebase option at Github.com. I’m not willing to get into the command line for something this insignificant. it’s crazy that it’s this difficult for such a tiny correction. — Kevin
On Jun 6, 2019, at 1:27 AM, Tom Hacohen notifications@github.com wrote:
Done, thanks!
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/tasn/webext-signed-pages/pull/21?email_source=notifications&email_token=ACSZARV4MD5XEFITVHLH2PLPZDDABA5CNFSM4HS7OES2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXCDYMQ#issuecomment-499399730, or mute the thread https://github.com/notifications/unsubscribe-auth/ACSZARQKOWC3Q3T6KFCGVTDPZDDABANCNFSM4HS7OESQ.
OK, sorry to hear. Please let me know if you ever get around to it, happy to accept the patch as discussed above. Though at least we now got the readme to show the correct commands. Thanks for that.
Updated the instructions with the yarn commands that worked for me when building the browser extension. The NPM commands didn't work for me:
yarn install yarn run build yarn run package