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.

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

Allow reaction linking from force #4498

Merged
merged 3 commits into from
Sep 9, 2019
Merged

Conversation

zephraph
Copy link
Contributor

@zephraph zephraph commented Sep 5, 2019

This is just the other direction from #4475. It does the same thing (links reaction into force), but it allows you do to so from force.

So you'd run

yarn integrate reaction

and that'd start up the linking process. I need to add some docs still.

@codecov
Copy link

codecov bot commented Sep 5, 2019

Codecov Report

Merging #4498 into master will increase coverage by 0.1%.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           master   #4498     +/-   ##
========================================
+ Coverage    79.1%   79.3%   +0.1%     
========================================
  Files        1270    1270             
  Lines       35045   35032     -13     
  Branches     2021    2022      +1     
========================================
+ Hits        27752   27781     +29     
+ Misses       6287    6260     -27     
+ Partials     1006     991     -15

cat package.json | jq -e .scripts.integrate &>2

if [ $? -ne 0 ]; then
echo "$PROJECT doesn't have integrate npm script, ensure it's valid"
Copy link
Member

Choose a reason for hiding this comment

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

Maybe reference something that would make it valid?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Like pointing to another project?

Copy link
Member

Choose a reason for hiding this comment

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

Or something -- a random dev won't know what "ensure it's valid' means.

Copy link
Contributor

Choose a reason for hiding this comment

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

And/or perhaps re-word it so people know to look at the scripts key of package.json? I’m not sure “npm scripts” is something everybody will immediately understand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the wording a little bit. I'm gonna write a blog post and some more detailed documentation on this process this week so I'll follow up once that's done.

@artsy-peril artsy-peril bot merged commit 28e5bac into master Sep 9, 2019
@artsy-peril artsy-peril bot mentioned this pull request Sep 9, 2019
@bhoggard bhoggard deleted the link-reaction-from-force branch April 8, 2020 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants