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

Hotfix/failing thin pull requests practice #207

Merged
merged 7 commits into from
Jan 21, 2020
+10 −10
Merged

Hotfix/failing thin pull requests practice #207

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c3e4ef4
fix: increase default allowed number of changes
adelkahomolova Jan 16, 2020
3533026
fix: save the right value to the id (prNumber)
adelkahomolova Jan 16, 2020
739354f
fix: tests according to change id value
adelkahomolova Jan 16, 2020
5fccdf9
fix: change id to number
adelkahomolova Jan 16, 2020
8ad09e0
fix: suggestion text
adelkahomolova Jan 20, 2020
61fdcde
docs: add comment to explain changed value of pr id
adelkahomolova Jan 20, 2020
153f153
Merge branch 'master' of https://github.com/DXHeroes/dx-scanner into …
adelkahomolova Jan 20, 2020
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
2 changes: 1 addition & 1 deletion src/inspectors/CollaborationInspector.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe('Collaboration Inspector', () => {
it('returns paginated pull requests', async () => {
new GitHubNock('1', 'octocat', 1296269, 'Hello-World').getPulls({
pulls: [
{ number: 1347, state: 'open', title: 'new-feature', body: 'Please pull these awesome changes', head: 'new-topic', base: 'master' },
{ number: 1, state: 'open', title: 'new-feature', body: 'Please pull these awesome changes', head: 'new-topic', base: 'master' },
],
});

Expand Down
2 changes: 1 addition & 1 deletion src/practices/LanguageIndependent/ThinPullRequestsPractice.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { Paginated } from '../../inspectors';
dependsOn: { practicing: ['LanguageIndependent.DoesPullRequests'] },
})
export class ThinPullRequestsPractice implements IPractice {
private readonly measurePullRequestCount = 500; // update suggestion text when changed
private readonly measurePullRequestCount = 1000; // update suggestion text when changed
prokopsimek marked this conversation as resolved.
Show resolved Hide resolved

async isApplicable(): Promise<boolean> {
return true;
Expand Down
6 changes: 3 additions & 3 deletions src/services/git/GitHubService.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ describe('GitHub Service', () => {
new GitHubNock('1', 'octocat', 1296269, 'Hello-World').getPulls({
pulls: [
{
number: 1347,
number: 1,
state: 'open',
title: 'new-feature',
body: 'Please pull these awesome changes',
Expand All @@ -69,7 +69,7 @@ describe('GitHub Service', () => {

it('returns pulls in own interface with diffStat', async () => {
const params = {
number: 1347,
number: 1,
state: 'open',
title: 'new-feature',
body: 'Please pull these awesome changes',
Expand Down Expand Up @@ -108,7 +108,7 @@ describe('GitHub Service', () => {
new GitHubNock('1', 'octocat', 1296269, 'Hello-World').getPulls({
pulls: [
{
number: 1347,
number: 1,
state: 'open',
title: 'new-feature',
body: 'Please pull these awesome changes',
Expand Down
6 changes: 3 additions & 3 deletions src/services/git/GitHubService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ export class GitHubService implements IVCSService {
closedAt: val.closed_at,
mergedAt: val.merged_at,
state: val.state,
id: val.id,
id: val.number,
base: {
repo: {
url: val.base.repo.url,
Expand All @@ -105,7 +105,7 @@ export class GitHubService implements IVCSService {
};
// Get number of changes, additions and deletions in PullRequest if the withDiffStat is true
if (options?.withDiffStat) {
const lines = await this.getPullsDiffStat(owner, repo, val.id);
const lines = await this.getPullsDiffStat(owner, repo, val.number);
return { ...pullRequest, lines };
}
return pullRequest;
Expand Down Expand Up @@ -136,7 +136,7 @@ export class GitHubService implements IVCSService {
closedAt: response.data.closed_at,
mergedAt: response.data.merged_at,
state: response.data.state,
id: response.data.id,
id: response.data.number,
Copy link
Member

@prokopsimek prokopsimek Jan 20, 2020 •
edited by adelkahomolova
Loading

Choose a reason for hiding this comment

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

How do you know that number attribute is an PR id?

  • Add a link referring to the information why the number is the correct PR id.

I have found just this:

Be aware that the id of a pull request returned from "Issues" endpoints will be an issue id. To find out the pull request id, use the "List pull requests" endpoint.

src.: https://developer.github.com/v3/issues/#list-issues
... but nothing about the number attribute.

base: {
repo: {
url: response.data.base.repo.url,
Expand Down
2 changes: 1 addition & 1 deletion src/services/git/__MOCKS__/gitHubServiceMockFolder/getPullsServiceResponse.mock.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export const getPullsServiceResponse: Paginated<PullRequest> = {
mergedAt: '2011-01-26T19:01:12Z',
state: 'open',
updatedAt: '2011-01-26T19:01:12Z',
url: 'https://api.github.com/repos/octocat/Hello-World/pulls/1347',
url: 'https://api.github.com/repos/octocat/Hello-World/pulls/1',
user: { id: '1', login: 'octocat', url: 'https://api.github.com/users/octocat' },
},
],
Expand Down