Skip to content

Conversation

@gr2m
Copy link

@gr2m gr2m commented Oct 31, 2018

closes #3

.post('/app/installations/321696/access_tokens')
.reply(200, {token: 'test'})

nock('https://api.github.com')
Copy link
Author

Choose a reason for hiding this comment

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

let me know if you want me to remove the nock chaining here. I just wanted to make sure you know about it :)

@gr2m
Copy link
Author

gr2m commented Oct 31, 2018

@hiimbex do you have CI setup yet?

Note that the tests pass although I didn’t implement the handling of the pull_request.synchronize action yet. This is confusing me, it looks like await probot.receive({name: 'pull_request', payload: payloadSynchronize}) does not check payload.action? Any idea?

@gr2m
Copy link
Author

gr2m commented Oct 31, 2018

For the record the "fix" is simple

-  app.on('pull_request.opened', async context => {
+  app.on([
+    'pull_request.opened',
+    'pull_request.synchronize'
+  ], async context => {

I just don’t want to push it so you can reproduce the tests already pass

Copy link
Owner

@hiimbex hiimbex left a comment

Choose a reason for hiding this comment

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

I'm not quite sure the fix is that simple...

My initial implementation takes the existing PR body and adds content to the end of it. If the bot has already editted it when the PR was first opened you could get a state like:

My pr body!!

----
Link to rendered markdown file: a

-----
Link to rendered markdown file b

Where a was edited when the PR was first opened, and b was editted and triggered from a synchronized event, or worse:

My pr body!!

----
Link to rendered markdown file: a

-----
Link to rendered markdown file a
Link to rendered markdown file b

where you edit a again.

I'm wondering in regards to the tests "passing" if because that payload is not an opened action if maybe it's not being run/triggered at all?

Also, yes should totally set up Travis! I'll get into that!

@hiimbex
Copy link
Owner

hiimbex commented Nov 1, 2018

Note that the tests pass although I didn’t implement the handling of the pull_request.synchronize action yet. This is confusing me, it looks like await probot.receive({name: 'pull_request', payload: payloadSynchronize}) does not check payload.action? Any idea?

After running the CI, I see I was wrong about it perhaps not being run at all. Honestly not sure! I'm guessing it's an issue with probot.receive and testing specific, not generally, but would be curious to investigate more, into receive:
https://github.com/probot/probot/blob/ad06bd5e321fdae893f88ef216102e0e330ba782/src/application.ts#L69-L81

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Listen to "synchronize" pull_request action

2 participants