[Dev] Update to Danger 2.0 #17234
Closed
[Dev] Update to Danger 2.0 #17234
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
the
CLA Signed
Signed-off-by: Orta Therox <orta@artsymail.com> Signed-off-by: Anandaroop Roy <roop@artsymail.com>
|
Looks great. I'll import it without landing first, as I believe this will require updating our internal repo's offline yarn cache. While danger is not used internally, we have some tests that ensure all dependencies in package.json are available offline. |
There was a problem hiding this comment.
@hramos has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
hramos
approved these changes
Dec 19, 2017
|
Thanks @hramos |
hramos
suggested changes
Dec 20, 2017
There was a problem hiding this comment.
After looking into what it'd take to land this internally, I'd prefer to keep Danger in its own directory for a few reasons:
- Dependencies in the root package.json need to be available in our internal offline cache, but we don't run danger internally as it's a GitHub-only concern
- We're using some additional dependencies in danger/package.json to help implement some of our rules. It wouldn't be much work to remove those dependencies and rewrite the rules, but we don't get much benefit from moving danger to the root of the repo so I am not sure it's necessary.
- We need to update several internal configs to ensure our linters, flow, etc ignore the dangerfile in its new location
If this were something we used internally, I'd continue working on getting this landed - I actually already did all the work. But I think we're in a good place with having danger in its own directory. I propose the following:
- Keep the dangerfile in
danger/for now. - Update to Danger 2 as done in this PR
- On another PR, consider renaming
danger/and using this directory for open-source-only scripts (similar to how we now haveflow-github/for OSS-only flow types). This way we can easily ignore anything OSS-only internally. We might be able to even use the existing.github/directory for this.
What do you think?
|
That makes sense to me 👍 |
Closed
|
Ah, this PR is still open, closing as #17310 did it |
mathiasbynens
pushed a commit
to mathiasbynens/react-native
that referenced
this pull request
Jan 13, 2018
Summary: See facebook#17234 for discussion. Closes facebook#17310 Differential Revision: D6627914 Pulled By: hramos fbshipit-source-id: 3d72fc69fe03c53706d2076ecf7b5321a00d1642
facebook-github-bot
pushed a commit
that referenced
this pull request
Nov 19, 2019
Summary: Changelog: [General][Changed] - React sync for revisions 0b61e2698...6cff70a74 (Includes React 16.11.0) This sync includes the following changes: - **[6cff70a74](facebook/react@6cff70a74 )**: [react-interactions] Expost host instance to Scope Query function (#17341) //<Dominic Gannaway>// - **[b8f825877](facebook/react@b8f825877 )**: Split ReactDOM entry point (#17331) //<Dan Abramov>// - **[a7b4d51a2](facebook/react@a7b4d51a2 )**: Warn when doing creat eRoot twice on the same node (another approach) (#17329) //<Dan Abramov>// - **[be3bfa6fa](facebook/react@be3bfa6fa )**: [Flight] Basic Integr ation Test (#17307) //<Dan Abramov>// - **[6cb6b1d66](facebook/react@6cb6b1d66 )**: Add yarn build --unsa fe-partial (#17316) //<Dan Abramov>// - **[38dd17ab9](facebook/react@38dd17ab9 )**: [RN] Hoist static dee pDiffer options object (#17303) //<Moti Zilberman>// - **[61d3dd0e0](facebook/react@61d3dd0e0 )**: Update deepDiffer usa ge in React Native renderer (#17282) //<Moti Zilberman>// - **[e701632ad](facebook/react@e701632ad )**: [react-interactions] Change unmount blur logic to a dedicated event (#17291) //<Dominic Gannaway>// - **[ce4b3e998](facebook/react@ce4b3e998 )**: [react-interactions] Add optional searchNodes to Scope.queryAllNodes (#17293) //<Dominic Gannaway>// - **[dee03049f](facebook/react@dee03049f )**: [Flight] Basic Streaming Suspense Support (#17285) //<Sebastian Markbåge>// - **[f50f39b55](facebook/react@f50f39b55 )**: [Flight] Better compat with http.createServer (#17289) //<Dan Abramov>// - **[345270630](facebook/react@345270630 )**: DevTools cleanup (#17283) //<Brian Vaughn>// - **[cd1bdcd06](facebook/react@cd1bdcd06 )**: [react-interactions] Prevent duplicate onPress firing for keyboard Enter (#17266) //<Dominic Gannaway>// - **[4f02c93c7](facebook/react@4f02c93c7 )**: Fix devtools displaying Anonymous for memo of ref-forwarding components (#17274) //<Waseem Dahman>// - **[053cf0fed](facebook/react@053cf0fed )**: Fix react-is memo and lazy type checks (#17278) //<Brian Vaughn>// - **[0f3838a01](facebook/react@0f3838a01 )**: Remove `debugRenderPhaseSideEffects` flag (#17270) //<Andrew Clark>// - **[cb09dbe0a](facebook/react@cb09dbe0a )**: [react-interactions] Add handleSimulateChildBlur upon DOM node removal (#17225) //<Dominic Gannaway>// - **[6095993d4](facebook/react@6095993d4 )**: Types: findHostInstance_DEPRECATED returns React.ElementRef<HostComponent<mixed>> (#17265) //<Eli White>// - **[62ef25077](facebook/react@62ef25077 )**: Avoid bundling in ponyfill for Object.assign in use-subscription package (#17259) //<Mateusz Burzyński>// - **[f4148b256](facebook/react@f4148b256 )**: [Flight] Move around the Server side a bit (#17251) //<Sebastian Markbåge>// - **[fadc97167](facebook/react@fadc97167 )**: [Flight] Add Client Infrastructure (#17234) //<Sebastian Markbåge>// - **[36fd29f09](facebook/react@36fd29f09 )**: Don't show empty (no work) commits in Profiler (#17253) //<Brian Vaughn>// - **[a2e05b6c1](facebook/react@a2e05b6c1 )**: [Scheduler] Delete old rAF implementation (#17252) //<Andrew Clark>// - **[6dc2734b4](facebook/react@6dc2734b4 )**: Codemod tests to `it.experimental` (#17243) //<Andrew Clark>// - **[273679a78](facebook/react@273679a78 )**: DevTools standalone shell changes: (#17213) //<Brian Vaughn>// - **[d0fc0ba0a](facebook/react@d0fc0ba0a )**: Revert "Dispatch commands to both UIManagers from both renderers (#17211)" (#17232) //<Eli White>// - **[bdcdb69a2](facebook/react@bdcdb69a2 )**: Rename findHostInstance_deprecated to findHostInstance_DEPRECATED (#17228) //<Eli White>// - **[515746c21](facebook/react@515746c21 )**: Add findHostInstance_deprecated to the React Native Renderer (#17224) //<Eli White>// - **[9a35adc96](facebook/react@9a35adc96 )**: Only call Profiler onRender when a descendant had work (#17223) //<Brian Vaughn>// - **[8eee0eb01](facebook/react@8eee0eb01 )**: Dispatch commands to both UIManagers from both renderers (#17211) //<Eli White>// - **[f4e974d26](facebook/react@f4e974d26 )**: Add Experimental Flight Infrastructure (#16398) //<Sebastian Markbåge>// - **[6cd365cac](facebook/react@6cd365cac )**: Don't treat the last row in hidden as deleted if already mounted (#17206) //<Sebastian Markbåge>// - **[048879eda](facebook/react@048879eda )**: [react-interactions] Ensure props on scope query function is always object (#17212) //<Dominic Gannaway>// - **[3497ccc14](facebook/react@3497ccc14 )**: Add guard to handle modified React elements with non-string keys (#17164) //<Brian Vaughn>// - **[3f9c03675](facebook/react@3f9c03675 )**: Typo fix in comment (#17111) //<Deniz Susman>// - **[f6b8d31a7](facebook/react@f6b8d31a7 )**: Rename createSyncRoot to createBlockingRoot (#17165) //<Dan Abramov>// - **[9c02d2654](facebook/react@9c02d2654 )**: docs: Fixed a typo in readme.md (#17119) //<Wilco Fiers>// - **[8075c8505](facebook/react@8075c8505 )**: Update local package versions for 16.10 release //<Andrew Clark>// - **[5faf377df](facebook/react@5faf377df )**: Fixed a style bug in props editor (#17162) //<Brian Vaughn>// - **[f7ec65eeb](facebook/react@f7ec65eeb )**: [react-interactions] Make events non-passive to allow preventDefault (#17136) //<Dominic Gannaway>// - **[1022ee0ec](facebook/react@1022ee0ec )**: Read current time without marking event start time (#17160) //<Andrew Clark>// - **[349cf5acc](facebook/react@349cf5acc )**: Experimental test helper: `it.experimental` (#17149) //<Andrew Clark>// - **[edc234c73](facebook/react@edc234c73 )**: Build script should default to experimental (#17144) //<Andrew Clark>// - **[3cc564547](facebook/react@3cc564547 )**: SuspenseList support in DevTools (#17145) //<Sebastian Markbåge>// - **[68fb58029](facebook/react@68fb58029 )**: Remove unstable_ prefix in various internal uses (#17146) //<Sebastian Markbåge>// - **[7082d5a2d](facebook/react@7082d5a2d )**: Don't build non-experimental www bundles (#17139) //<Andrew Clark>// - **[c47f59331](facebook/react@c47f59331 )**: Move SuspenseList to experimental package (#17130) //<Andrew Clark>// - **[685ed561f](facebook/react@685ed561f )**: Migrate useDeferredValue and useTransition (#17058) //<Luna Ruan>// - **[0b61e2698](facebook/react@0b61e2698 )**: Update RN typings for a shim (#17138) //<Dan Abramov>// Reviewed By: threepointone Differential Revision: D18428149 fbshipit-source-id: 28273be4d7a4c7ec0fe0451cea134ee09a3b4d86
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Danger 2.0 is pretty stable now (Apollo-client just updated to 2.0, so I was reminded of RN) and more importantly for RN, it doesn't depends on jest's internals now. This means Danger doesn't need special casing like before.
/cc @hramos @grabbou
Test Plan
I ran Danger PR to validate that any PR runs:
I expect the CI will tell me whether it works or not
Related PRs
#16350