Skip to content
This repository has been archived by the owner on Mar 12, 2020. It is now read-only.

Update Dangerfile for noting tests, and stories on a PR #457

Merged
merged 2 commits into from
Mar 21, 2017

Conversation

orta
Copy link
Contributor

@orta orta commented Mar 16, 2017

Adds the following:

  • Any touched (new/modified) react components warn if they are not somewhere inside the storybook stories
  • Any touched app files fails if there are no corresponding test files (not changes) - you need to specify that you are not writing tests in the PR body/title

WIP as I'm not sure it'll compile etc, I did it offline

@orta orta changed the title [WIP] Update Dangerfile for noting tests, and stories on a PR Update Dangerfile for noting tests, and stories on a PR Mar 18, 2017
@orta
Copy link
Contributor Author

orta commented Mar 21, 2017

Performance in tests is pretty terrible

screen shot 2017-03-21 at 10 12 08

I thought this was Danger's fault initially, but it isn't (older pr) - looks like a side-effect of the TS conversion.

@orta
Copy link
Contributor Author

orta commented Mar 21, 2017

Bet this is the haste map generation - running tests twice "fixes it"

screen shot 2017-03-21 at 10 14 04

@alloy
Copy link
Contributor

alloy commented Mar 21, 2017

Can that be added to Travis’ cache?

@orta
Copy link
Contributor Author

orta commented Mar 21, 2017

Yeah, they're in a determinate location, I've had to find it before

@orta
Copy link
Contributor Author

orta commented Mar 21, 2017

Here we are: cacheDirectory

dangerfile.ts Outdated
fail("No CHANGELOG added.")
}

// No PR is too small to warrent a paragraph or two of summary
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: warrant

dangerfile.ts Outdated
warn(`${message} - <i>${idea}</i>`)
}

// Always ensure we assign someone, so that our Slackbot can do it's work correctly
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: its

@alloy
Copy link
Contributor

alloy commented Mar 21, 2017

Good stuff 👌

@orta
Copy link
Contributor Author

orta commented Mar 21, 2017

Nits picked, and made a separate PR for jest cache.

@orta
Copy link
Contributor Author

orta commented Mar 21, 2017

This just needs to wait on 0.14.1 - in putting this in a real project I found a bug, after that - good to go

@orta
Copy link
Contributor Author

orta commented Mar 21, 2017

Deployment is via CI, so is indeterminate - you know what that's like ;)

@alloy
Copy link
Contributor

alloy commented Mar 21, 2017

Ok, can you label this as WIP until that’s released?

@orta
Copy link
Contributor Author

orta commented Mar 21, 2017

shipped ;)

@alloy
Copy link
Contributor

alloy commented Mar 21, 2017

Hah, I see now what you meant by “deployment is via CI” :D

@orta
Copy link
Contributor Author

orta commented Mar 21, 2017

OK 👍 this is good to go

@alloy alloy merged commit 99c58ef into master Mar 21, 2017
@alloy alloy deleted the update_jest_danger branch March 21, 2017 10:48
@@ -6,6 +6,7 @@
"rootDir": "src",
"outDir": "build",
"allowJs": true,
"pretty": true,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh yeah, this just provides some extra colours when you see typescript errors in the console.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants