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
Merged

Hotfix/failing thin pull requests practice #207

Show file tree
Hide file tree
Changes from all 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
4 changes: 2 additions & 2 deletions src/practices/LanguageIndependent/ThinPullRequestsPractice.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ import { Paginated } from '../../inspectors';
name: 'Break down large pull requests into smaller ones',
impact: PracticeImpact.medium,
suggestion:
'Large pull request are hard to code review and it reduces the probability of finding bugs. Split your PRs into logical units. Do not have PR with more than 500 changes.',
'Large pull request are hard to code review and it reduces the probability of finding bugs. Split your PRs into logical units. Do not have PR with more than 1000 changes.',
reportOnlyOnce: true,
url: 'https://medium.com/@hugooodias/the-anatomy-of-a-perfect-pull-request-567382bb6067',
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, // Lists details of a pull request by providing its number. - https://developer.github.com/v3/pulls/
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, // Lists details of a pull request by providing its number. - https://developer.github.com/v3/pulls/
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