Skip to content

Setting up api#4

Open
mKleinCreative wants to merge 9 commits into
masterfrom
setting-up-api
Open

Setting up api#4
mKleinCreative wants to merge 9 commits into
masterfrom
setting-up-api

Conversation

@mKleinCreative

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread src/server/routes/auth.js Outdated
@@ -0,0 +1,23 @@
const apiClient = require('../../gateway/codeship');

@punitrathore punitrathore Oct 18, 2017

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

since this is a file inside the routes folder it should return a router

Comment thread src/gateway/codeship.js Outdated
const password = process.env.CODESHIP_PASSWORD;
let authorization;

async function postAuth() {

@punitrathore punitrathore Oct 18, 2017

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

rename the function to authenticate

Comment thread src/gateway/codeship.js Outdated
return authorization;
}

function listProjects(organizationId) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

rename function to getProjects

Comment thread src/gateway/codeship.js Outdated
});
}

function listBuilds(organizationId, projectId) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

rename function to getBuilds, also check how you can only fetch the last build as opposed to all of them?

Comment thread src/gateway/codeship.js Outdated
@@ -0,0 +1,61 @@
require('dotenv').load();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this should be moved to the config.js file, and should only happen when the NODE_ENV is development

Comment thread src/gateway/codeship.js Outdated
const apiUrl = process.env.CODESHIP_API_URL;
const username = process.env.CODESHIP_USERNAME;
const password = process.env.CODESHIP_PASSWORD;
let authorization;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

rename to authenticationResponse.

Comment thread src/server/routes/auth.js Outdated
let projects = (await apiClient.listProjects(orgId)).projects;

// Embed the latest builds into each project
projects = await Promise.all(projects.map(async (project) => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

extract this function, and name it addBuildsToProject

const router = require('express').Router();

router.get('/', async (req, res) => {
async function appendBuildsToProjects(project) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

can you move this function out of the router function?

@punitrathore punitrathore left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

please look at the final comment :)

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