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

Improve repo-local packages implementation #17892

Merged
merged 3 commits into from Aug 21, 2018

Conversation

Projects
None yet
2 participants
@daviwil
Member

daviwil commented Aug 21, 2018

Description of the Change

This PR is a continuation of PR #17686 to address some feedback about how we detect the dev resource path in main.js. It streamlines the dev resource path detection process so that it's only performed once, cutting the number of synchronous file system checks in half.

Also included is a removal of the separate repo-local package installation step in script/bootstrap. It turns out that the more recent npm version used in apm causes a symbolic link to be created to the local package path, resulting in the package dependencies being installed simultaneously in the repo-local package path and Atom's node_modules.

Benefits

  • No unnecessary synchronous filesystem checks on startup
  • No unnecessary consolidated core package installations

Verification Process

  • atom --dev . uses ATOM_DEV_RESOURCE_PATH if one is set
  • atom --dev . in Atom repo path uses current directory as dev resource path when there is no ATOM_DEV_RESOURCE_PATH set and the current directory appears to be a clone of atom/atom
  • atom --dev . uses ~/github/atom as the dev resource path if it exists and none of the path sources above are present
  • atom --dev . uses its bundled resource path when none of the path sources above are present
  • atom -r path/to/atom overrides all other possible resource path sources

Applicable Issues

#17686

@daviwil daviwil requested a review from maxbrunsfeld Aug 21, 2018

@maxbrunsfeld

Awesome!

@daviwil daviwil merged commit cbcad27 into master Aug 21, 2018

3 checks passed

VSTS: Atom Pull Requests 20180821.10 succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@daviwil daviwil deleted the dw-improve-repo-local-packages branch Aug 21, 2018

@Arcanemagus Arcanemagus referenced this pull request Sep 15, 2018

Closed

Crash in directories with invalid package.json #18058

1 of 1 task complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment