Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

Commit

Permalink
Removing lodash as a dependency (#2435)
Browse files Browse the repository at this point in the history
* Removing `lodash` as a dependency

We're only depending on `flowRight`, so these changes remove
the dependency on `lodash`, switching the dependency to
`lodash.flowright`. React Apollo 3.0 will be removing this
dependency completely, but for now this will help with bundle
sizes.

All development `lodash` use has also been adjusted to use
individually imported `lodash` modules. Those will likely be
removed as well in React Apollo 3.0.

* Fix module name typo

* Fix file line endings

* Changelog update
  • Loading branch information
hwillson authored Sep 28, 2018
1 parent a90e815 commit 4eebfdc
Show file tree
Hide file tree
Showing 8 changed files with 354 additions and 316 deletions.
7 changes: 6 additions & 1 deletion Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,13 @@

## vNext

- Fix `lodash` typings. <br/>
- Fix `lodash` typings. <br/>
[@williamboman](https://github.com/williamboman) in [#2430](https://github.com/apollographql/react-apollo/pull/2430)
- Replace the `lodash` dependency with `lodash.flowright` (since that's the
only non-dev `lodash` function we're dependent on). Dev `lodash`
dependencies have also been updated to use their individual module
equivalent. <br/>
[@hwillson](https://github.com/hwillson) in [#2435](https://github.com/apollographql/react-apollo/pull/2435)

## 2.2.2 (September 28, 2018)

Expand Down
200 changes: 100 additions & 100 deletions dangerfile.ts
Original file line number Diff line number Diff line change
@@ -1,100 +1,100 @@
// Removed import
import includes from 'lodash/includes';
import fs from 'fs';

// Setup
const pr = danger.github.pr;
const commits = danger.github.commits;
const modified = danger.git.modified_files;
const bodyAndTitle = (pr.body + pr.title).toLowerCase();

// Custom modifiers for people submitting PRs to be able to say "skip this"
const trivialPR = bodyAndTitle.includes('trivial');
const acceptedNoTests = bodyAndTitle.includes('skip new tests');

const typescriptOnly = (file: string) => includes(file, '.ts');
const filesOnly = (file: string) => fs.existsSync(file) && fs.lstatSync(file).isFile();

// Custom subsets of known files
const modifiedAppFiles = modified
.filter(p => includes(p, 'src/'))
.filter(p => filesOnly(p) && typescriptOnly(p));

const modifiedTestFiles = modified.filter(p => includes(p, 'test/'));

// Takes a list of file paths, and converts it into clickable links
const linkableFiles = paths => {
const repoURL = danger.github.pr.head.repo.html_url;
const ref = danger.github.pr.head.ref;
const links = paths.map(path => {
return createLink(`${repoURL}/blob/${ref}/${path}`, path);
});
return toSentence(links);
};

// ["1", "2", "3"] to "1, 2 and 3"
const toSentence = (array: Array<string>): string => {
if (array.length === 1) {
return array[0];
}
return array.slice(0, array.length - 1).join(', ') + ' and ' + array.pop();
};

// ("/href/thing", "name") to "<a href="/href/thing">name</a>"
const createLink = (href: string, text: string): string => `<a href='${href}'>${text}</a>`;

// Raise about missing code inside files
const raiseIssueAboutPaths = (type: Function, paths: string[], codeToInclude: string) => {
if (paths.length > 0) {
const files = linkableFiles(paths);
const strict = '<code>' + codeToInclude + '</code>';
type(`Please ensure that ${strict} is enabled on: ${files}`);
}
};

const authors = commits.map(x => x.author && x.author.login);
const isBot = authors.some(x => ['greenkeeper', 'renovate'].indexOf(x) > -1);

if (!isBot) {
// Rules
// When there are app-changes and it's not a PR marked as trivial, expect
// there to be CHANGELOG changes.
const changelogChanges = includes(modified, 'Changelog.md');
if (modifiedAppFiles.length > 0 && !trivialPR && !changelogChanges) {
fail('No CHANGELOG added.');
}

// No PR is too small to warrant a paragraph or two of summary
if (pr.body.length === 0) {
fail('Please add a description to your PR.');
}

const hasAppChanges = modifiedAppFiles.length > 0;

const hasTestChanges = modifiedTestFiles.length > 0;

// Warn when there is a big PR
const bigPRThreshold = 500;
if (danger.github.pr.additions + danger.github.pr.deletions > bigPRThreshold) {
warn(':exclamation: Big PR');
}

// Warn if there are library changes, but not tests
if (hasAppChanges && !hasTestChanges) {
warn(
"There are library changes, but not tests. That's OK as long as you're refactoring existing code",
);
}

// Be careful of leaving testing shortcuts in the codebase
const onlyTestFiles = modifiedTestFiles.filter(x => {
const content = fs.readFileSync(x).toString();
return (
content.includes('it.only') ||
content.includes('describe.only') ||
content.includes('fdescribe') ||
content.includes('fit(')
);
});
raiseIssueAboutPaths(fail, onlyTestFiles, 'an `only` was left in the test');
}
// Removed import
import includes from 'lodash.includes';
import fs from 'fs';

// Setup
const pr = danger.github.pr;
const commits = danger.github.commits;
const modified = danger.git.modified_files;
const bodyAndTitle = (pr.body + pr.title).toLowerCase();

// Custom modifiers for people submitting PRs to be able to say "skip this"
const trivialPR = bodyAndTitle.includes('trivial');
const acceptedNoTests = bodyAndTitle.includes('skip new tests');

const typescriptOnly = (file: string) => includes(file, '.ts');
const filesOnly = (file: string) => fs.existsSync(file) && fs.lstatSync(file).isFile();

// Custom subsets of known files
const modifiedAppFiles = modified
.filter(p => includes(p, 'src/'))
.filter(p => filesOnly(p) && typescriptOnly(p));

const modifiedTestFiles = modified.filter(p => includes(p, 'test/'));

// Takes a list of file paths, and converts it into clickable links
const linkableFiles = paths => {
const repoURL = danger.github.pr.head.repo.html_url;
const ref = danger.github.pr.head.ref;
const links = paths.map(path => {
return createLink(`${repoURL}/blob/${ref}/${path}`, path);
});
return toSentence(links);
};

// ["1", "2", "3"] to "1, 2 and 3"
const toSentence = (array: Array<string>): string => {
if (array.length === 1) {
return array[0];
}
return array.slice(0, array.length - 1).join(', ') + ' and ' + array.pop();
};

// ("/href/thing", "name") to "<a href="/href/thing">name</a>"
const createLink = (href: string, text: string): string => `<a href='${href}'>${text}</a>`;

// Raise about missing code inside files
const raiseIssueAboutPaths = (type: Function, paths: string[], codeToInclude: string) => {
if (paths.length > 0) {
const files = linkableFiles(paths);
const strict = '<code>' + codeToInclude + '</code>';
type(`Please ensure that ${strict} is enabled on: ${files}`);
}
};

const authors = commits.map(x => x.author && x.author.login);
const isBot = authors.some(x => ['greenkeeper', 'renovate'].indexOf(x) > -1);

if (!isBot) {
// Rules
// When there are app-changes and it's not a PR marked as trivial, expect
// there to be CHANGELOG changes.
const changelogChanges = includes(modified, 'Changelog.md');
if (modifiedAppFiles.length > 0 && !trivialPR && !changelogChanges) {
fail('No CHANGELOG added.');
}

// No PR is too small to warrant a paragraph or two of summary
if (pr.body.length === 0) {
fail('Please add a description to your PR.');
}

const hasAppChanges = modifiedAppFiles.length > 0;

const hasTestChanges = modifiedTestFiles.length > 0;

// Warn when there is a big PR
const bigPRThreshold = 500;
if (danger.github.pr.additions + danger.github.pr.deletions > bigPRThreshold) {
warn(':exclamation: Big PR');
}

// Warn if there are library changes, but not tests
if (hasAppChanges && !hasTestChanges) {
warn(
"There are library changes, but not tests. That's OK as long as you're refactoring existing code",
);
}

// Be careful of leaving testing shortcuts in the codebase
const onlyTestFiles = modifiedTestFiles.filter(x => {
const content = fs.readFileSync(x).toString();
return (
content.includes('it.only') ||
content.includes('describe.only') ||
content.includes('fdescribe') ||
content.includes('fit(')
);
});
raiseIssueAboutPaths(fail, onlyTestFiles, 'an `only` was left in the test');
}
Loading

0 comments on commit 4eebfdc

Please sign in to comment.