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

Upgrade apm #19673

Merged
merged 2 commits into from Jul 17, 2019

Conversation

@nathansobo
Copy link
Contributor

commented Jul 15, 2019

This PR upgrades apm to include atom/apm#854.

This is a sensitive component, so I want a green build before disrupting master.

@Aerijo

This comment has been minimized.

Copy link
Member

commented Jul 15, 2019

@nathansobo The problem is more than this actually; e.g., apm remove .. will delete the whole .atom folder.

Suggestions:

  • Validate the folder is a package.
  • Enforce that the resolved path is a folder inside the packages or dev/packages folder.
  • Just enforce the parameter be a package name, not a path (npm seems to do this). And the apm help text describes it as <package_name>, so maybe path support itself can be considered a bug?
@nathansobo

This comment has been minimized.

Copy link
Contributor Author

commented Jul 16, 2019

@Aerijo great point. I opened atom/apm#858 to try to defend against weird edge cases that resolve to a non-package folder. I'll hold off on merging this until that solution is published as well.

@nathansobo nathansobo merged commit e9a0f3e into master Jul 17, 2019

1 check passed

Atom Pull Requests #20190716.4 succeeded
Details

@nathansobo nathansobo deleted the ns/upgrade-apm branch Jul 17, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.