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

Eliminate trivial underscore #147

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Eliminate trivial underscore #147

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
28a3f0f
Changing own request.js to async interface
2colours Aug 31, 2024
358d44e
Remove uses of underscore where it's obvious
2colours Dec 1, 2024
310756f
Review related fixes
2colours Dec 22, 2024
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
42 changes: 18 additions & 24 deletions src/develop.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,31 +43,25 @@ cmd-shift-o to run the package out of the newly cloned repository.\
return options.alias('h', 'help').describe('help', 'Print this usage message');
}

getRepositoryUrl(packageName) {
return new Promise((resolve, reject) => {
const requestSettings = {
url: `${config.getAtomPackagesUrl()}/${packageName}`,
json: true
};
return request.get(requestSettings, (error, response, body) => {
body ??= {};
if (error != null) {
return void reject(`Request for package information failed: ${error.message}`);
}

if (response.statusCode === 200) {
const repositoryUrl = body.repository.url;
if (repositoryUrl) {
return void resolve(repositoryUrl);
}

return void reject(`No repository URL found for package: ${packageName}`);
}
async getRepositoryUrl(packageName) {
const requestSettings = {
url: `${config.getAtomPackagesUrl()}/${packageName}`,
json: true
};
const response = await request.get(requestSettings).catch(error => Promise.reject(`Request for package information failed: ${error.message}`));
const body = response.body ?? {};

if (response.statusCode === 200) {
const repositoryUrl = body.repository.url;
if (repositoryUrl) {
return repositoryUrl;
}

throw `No repository URL found for package: ${packageName}`;
}

const message = request.getErrorMessage(body, error);
return void reject(`Request for package information failed: ${message}`);
});
});
const message = request.getErrorMessage(body, error);
throw `Request for package information failed: ${message}`;
}

async cloneRepository(repoUrl, packageDirectory, options) {
Expand Down
24 changes: 10 additions & 14 deletions src/featured.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,29 +28,25 @@ List the Pulsar packages and themes that are currently featured.\
return options.boolean('json').describe('json', 'Output featured packages as JSON array');
}

getFeaturedPackagesByType(atomVersion, packageType) {
async getFeaturedPackagesByType(atomVersion, packageType) {

const requestSettings = {
url: `${config.getAtomApiUrl()}/${packageType}/featured`,
json: true
};
if (atomVersion) { requestSettings.qs = {engine: atomVersion}; }

return new Promise((resolve, reject) => void request.get(requestSettings, function(error, response, body) {
body ??= [];
if (error != null) {
return void reject(error);
}
if (response.statusCode === 200) {
let packages = body.filter(pack => (pack != null ? pack.releases : undefined) != null);
const response = await request.get(requestSettings);
const body = response.body ?? [];
if (response.statusCode === 200) {
let packages = body.filter(pack => pack?.releases != null);
packages = packages.map(({readme, metadata, downloads, stargazers_count}) => _.extend({}, metadata, {readme, downloads, stargazers_count}));
packages = _.sortBy(packages, 'name');
return void resolve(packages);
}

const message = request.getErrorMessage(body, error);
reject(`Requesting packages failed: ${message}`);
}));
return packages;
}

const message = request.getErrorMessage(body, error);
throw `Requesting packages failed: ${message}`;
}

async getAllFeaturedPackages(atomVersion) {
Expand Down
34 changes: 14 additions & 20 deletions src/install.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,32 +209,26 @@ Run ppm -v after installing Git to see what version has been detected.\
// packageName - The string name of the package to request.
//
// return value - A Promise that rejects with an appropriate error or resolves to the response body
requestPackage(packageName) {
async requestPackage(packageName) {
const requestSettings = {
url: `${config.getAtomPackagesUrl()}/${packageName}`,
json: true,
retries: 4
};
return new Promise((resolve, reject) => {
request.get(requestSettings, (error, response, body) => {
let message;
body ??= {};
if (error != null) {
message = `Request for package information failed: ${error.message}`;
if (error.status) { message += ` (${error.status})`; }
return void reject(message);
}
if (response.statusCode !== 200) {
message = request.getErrorMessage(body, error);
return void reject(`Request for package information failed: ${message}`);
}
if (!body.releases.latest) {
return void reject(`No releases available for ${packageName}`);
}

resolve(body);
});
const response = await request.get(requestSettings).catch(error => {
const message = request.getErrorMessage(body, error);
Copy link
Member

Choose a reason for hiding this comment

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

The first parameter here body doesn't exist, as we are only inside the catch() callback.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I feel this was "too clever". I rather don't want to change the logic here at the moment, I will revert to the original way of producing the error message here.

throw `Request for package information failed: ${message}`;
});
const body = response.body ?? {};
if (response.statusCode !== 200) {
const message = request.getErrorMessage(body, null);
throw `Request for package information failed: ${message}`;
}
if (!body.releases.latest) {
throw `No releases available for ${packageName}`;
}

return body;
}

// Is the package at the specified version already installed?
Expand Down
91 changes: 30 additions & 61 deletions src/publish.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,35 +105,24 @@ have published it.\
// return value - A Promise that resolves (without a value) when either the
// number of max retries have been reached or the tag could
// actually be retrieved.
waitForTagToBeAvailable(pack, tag) {
let retryCount = 5;
async waitForTagToBeAvailable(pack, tag) {
const retryCount = 5;
const interval = 1000;
const requestSettings = {
url: `https://api.github.com/repos/${Packages.getRepository(pack)}/tags`,
json: true
};

return new Promise((resolve, _reject) => {
const requestTags = () => {
request.get(
requestSettings,
(_error, response, tags) => {
tags ??= [];
if (response?.statusCode === 200) {
if (tags.some(t => t.name === tag)) {
resolve();
return;
}
}
if (--retryCount <= 0) {
return void resolve();
}
setTimeout(requestTags, interval);
}
);
};
requestTags();
});
for (let i = 0; i < retryCount; i++) {
const response = await request.get(requestSettings).catch();
const tags = response.body ?? [];
if (response?.statusCode === 200) {
if (tags.some(t => t.name === tag)) {
return;
}
}
await new Promise(resolve => setTimeout(resolve, interval)); //not strictly necessary on the last iteration
}
}

// Does the given package already exist in the registry?
Expand All @@ -151,15 +140,9 @@ have published it.\
authorization: token
}
};
return new Promise((resolve, reject) => {
request.get(requestSettings, (error, response, body) => {
body ??= {};
if (error != null) {
return void reject(error);
}
resolve(response.statusCode === 200);
});
});
const response = await request.get(requestSettings);
const body = response.body ?? {};
return response.statusCode === 200;
}

// Register the current repository with the package registry.
Expand Down Expand Up @@ -197,22 +180,16 @@ have published it.\
authorization: token
}
};
return new Promise((resolve, reject) => {
request.post(requestSettings, (error, response, body) => {
body ??= {};
if (error != null) {
return void reject(error);
}
if (response.statusCode !== 201) {
const message = request.getErrorMessage(body, error);
this.logFailure();
return void reject(`Registering package in ${repository} repository failed: ${message}`);
}
const response = await request.post(requestSettings);
const body = response.body ?? {};
if (response.statusCode !== 201) {
const message = request.getErrorMessage(body, null);
this.logFailure();
throw `Registering package in ${repository} repository failed: ${message}`; //again, why the double logging?
}

this.logSuccess();
return resolve(true);
});
});
this.logSuccess();
return true;
} catch (error) {
this.logFailure();
throw error;
Expand All @@ -238,20 +215,12 @@ have published it.\
authorization: token
}
};
return new Promise((resolve, reject) => {
request.post(requestSettings, (error, response, body) => {
body ??= {};
if (error != null) {
return void reject(error);
}
if (response.statusCode !== 201) {
const message = request.getErrorMessage(body, error);
return void reject(`Creating new version failed: ${message}`);
}

resolve();
});
});
const response = await request.post(requestSettings);
const body = response.body ?? {};
if (response.statusCode !== 201) {
const message = request.getErrorMessage(body, null);
throw `Creating new version failed: ${message}`;
}
}

// Publish the version of the package associated with the given tag.
Expand Down
Loading