Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes for devEnvSetup.js #23

Merged
merged 10 commits into from
Sep 29, 2023
Merged

Fixes for devEnvSetup.js #23

merged 10 commits into from
Sep 29, 2023

Conversation

dd137
Copy link

@dd137 dd137 commented Sep 21, 2023

The script crashes due to

  • .bin/npm not being available in all packages installed by devEnvSetup itself
  • Trying to run tests for a meteor-desktop-localstorage package, even though that package isn't installed by devEnvSetup

The script also ignores the user's answer about whether to run tests or not (it always runs them, possibly after executing finish()). For convenience, it would also be nice to know what are the default answers to the script's questions.

This PR addresses these small problems, along with a rewrite of the related README section about "Developing meteor-desktop" and using devEnvSetup.

Note: The last two tests run by devEnvSetup, npm run desktop -- build -b and npm run test-desktop in meteor-desktop-test-app also fail, but I don't think this is caused by devEnvSetup itself and therefore is not addressed by this PR.

Tested with meteor-desktop 3.1.0.

@StorytellerCZ
Copy link
Member

@dd137 thank you for this PR. I think this is a general improvement over the existing situation, so unless you still want to add something I would like to merge this and then continue on from there.

@StorytellerCZ StorytellerCZ changed the base branch from master to fix/bug-12 September 26, 2023 13:21
@StorytellerCZ
Copy link
Member

Changed the target towards the fix branch.

@StorytellerCZ
Copy link
Member

@dd137 do you want to do anything more here.

@dd137
Copy link
Author

dd137 commented Sep 26, 2023

@StorytellerCZ Thanks. I realized test-integration is missing from devEnvSetup.js. I'll add that with one more commit then I'm done here.

@dd137
Copy link
Author

dd137 commented Sep 28, 2023

@StorytellerCZ Feel free to review and merge this PR. Thank you!

@dd137 dd137 marked this pull request as ready for review September 28, 2023 13:29
@StorytellerCZ StorytellerCZ merged commit 9b56f3e into fix/bug-12 Sep 29, 2023
1 check failed
@StorytellerCZ StorytellerCZ deleted the fix/devEnvSetup branch September 29, 2023 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants