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.

Sign up for GitHub

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

src: Rebrand two lines of "ppm --version" output #100

Merged
merged 5 commits into from
Oct 1, 2023
Merged

src: Rebrand two lines of "ppm --version" output #100

Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
52a0c20
src: Rebrand "ppm --version" ("atom" --> "pulsar")
DeeDeeG Sep 25, 2023
47d0426
src: Rebrand "ppm --version" ("apm" --> "ppm")
DeeDeeG Sep 25, 2023
6119199
src: Restore atom and apm properties to JSON output of --version
DeeDeeG Oct 1, 2023
6ceabf1
spec: Rename apm-cli.js to apm-cli-spec.js
DeeDeeG Oct 1, 2023
0d7bf2a
spec: Adjust expected output for ppm --version
DeeDeeG Oct 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/apm-cli.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,10 @@ const printVersions = function(args, callback) {
let versions;
if (args.json) {
versions = {
apm: apmVersion,
ppm: apmVersion,
npm: npmVersion,
node: nodeVersion,
atom: atomVersion,
pulsar: atomVersion,
python: pythonVersion,
git: gitVersion,
nodeArch: process.arch
Copy link
Member Author

@DeeDeeG DeeDeeG Sep 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. About this JSON output.

Wondering if I need to stuff apm and atom into the JSON output, since that is presumably meant for programmatic use. I'm admittedly not familiar with how this JSON output is used, but wouldn't be at all surprised if this is relevant to, say, checking if the user's installed atomVersion is a supported version in a package-to-be-installed's engines field in its package.json?

Or this PR could possibly just leave the JSON output not re-branded at all, leave the interface for programmatic use totally 100% unchanged. (Adding new properties seems safe-ish to me, deleting atom and apm might be disruptive either to core editor if it's used there (not sure) and probably has some impact when interacting with the package backend??)

Copy link
Member Author

@DeeDeeG DeeDeeG Sep 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

./bin/ppm install still works on this branch, though, as does ./bin/ppm install --json. So, I haven't found a place where this clearly breaks stuff with just trying ppm install so far.

ToDo: I guess I could build Pulsar locally with ppm carrying this change and see how broken (if at all) package management is in the core editor??

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could do all of that, although if it were me, I'd opt to just adding new properties, if it is something you are worried about.

Expand All @@ -133,10 +133,10 @@ const printVersions = function(args, callback) {
if (gitVersion == null) { gitVersion = ''; }
if (atomVersion == null) { atomVersion = ''; }
versions = `\
${'apm'.red} ${apmVersion.red}
${'ppm'.red} ${apmVersion.red}
${'npm'.green} ${npmVersion.green}
${'node'.blue} ${nodeVersion.blue} ${process.arch.blue}
${'atom'.cyan} ${atomVersion.cyan}
${'pulsar'.cyan} ${atomVersion.cyan}
${'python'.yellow} ${pythonVersion.yellow}
${'git'.magenta} ${gitVersion.magenta}\
`;
Expand Down