Skip to content
This repository has been archived by the owner on May 30, 2024. It is now read-only.

post a message on conflicting PRS #1

Closed
ericLemanissier opened this issue Nov 12, 2020 · 11 comments
Closed

post a message on conflicting PRS #1

ericLemanissier opened this issue Nov 12, 2020 · 11 comments
Assignees

Comments

@ericLemanissier
Copy link
Owner

No description provided.

@ericLemanissier ericLemanissier self-assigned this Nov 12, 2020
@madebr
Copy link

madebr commented Nov 14, 2020

This would be useful!
I've got some questions about the intended behaviour:

  1. What should the bot do when a conflict is resolved?
  2. What should the bot do when a previously-commented-on issue has new conflicts?

Having the same behaviour as conan-center-bot would be the most verbose and clearest to the pr creator.
But this can lead to an overload of comments.
Editing previous comment instead of creating a new one is the densest but can be overlooked.
Anyway it would be nice to hide older comments (and hope @conan-center-bot will adopt this behaviour).

Maybe you should create a new @EricLemanissierBot user account and let the bot run using that account.
Same way as @Croydon does with his @CroydonBot .
That would avoid the risk of the bot mixing up your messages with its messages.

@ericLemanissier
Copy link
Owner Author

I'm exploring API, but unfortunately it does not seem to be possible to hide issue comments : https://docs.github.com/en/free-pro-team@latest/rest/reference/issues#update-an-issue-comment. The closest solution would be to delete the issue comment

@madebr
Copy link

madebr commented Nov 15, 2020

FYI, instead of using requests, you can also use pygithub.
I've used it in the past and found it pleasant to use.

@madebr
Copy link

madebr commented Nov 15, 2020

I've searched on this and there exists a graphql api at https://docs.github.com/en/free-pro-team@latest/graphql
This API supports minimization of comments
pygithub only supports api v3, I think graphql is api v4

@madebr
Copy link

madebr commented Nov 15, 2020

Congratulations on creating the bot @ericLemanissierBot :D
conan-io/conan-center-index#2867 (comment)

I think there is a grammar error: There seems to be
Or re-word the comment.
e.g.

I detected other pull requests that are modifying the same recipe:
- ...
- ...
- ...

This message was auto-generated by a bot.
Please complain at https://github.com/ericLemanissier/conan-center-conflicting-prs/issues

ericLemanissier added a commit that referenced this issue Nov 16, 2020
@ericLemanissier
Copy link
Owner Author

Thanks for pushing me on this one. As a first step, I made sure that there is only one message of the bot per PR, and the message is updated in case the conflict changes.
The problem with doing something when the conflict is resolved, is that the bot needs to have some durable state in order to know that there was a conflict before. Currently, the only state we have is the content of the table in conan-io/conan-center-index#3571 (comment), and I'm not very confident that parsing this table is a good table. Do you have an idea ?
Thanks for the info about graphql. I didn't know about it yet. I'm not sure I want to dive into this just for the "hide comment" feature though :/. It seems a very significant shift from REST api

@prince-chrismc
Copy link

I absolutely hate this 😃 please stop spamming my inbox!

@madebr
Copy link

madebr commented Nov 16, 2020

Ok, we got the first complaint 😄
So I guess we should edit the comments.
Ideally, @conan-center-bot would tag the pr's.

You have some options with retaining state between actions:

  1. use github actions functionality:
  • pro: you use github actions all the way, file oriented
  • con: very github focused, unclear to me where the data is stored
  1. store the data, encoded as e.g. json/yml, in a comment somewhere and edit the comment every time
  • pro: the data is there for you to manually check
  • con: no complex data structures (but that is not a problem here)
  1. store the data in a (orphaned) branch. This combines the pros of 1 and 2 without the con of 1 and 2.

@ericLemanissier
Copy link
Owner Author

The 3rd option seems good, but to be honest, I don't think I'll take the time to learn the graphql api (which I know nothing about), implement a database file, and make the bot maintain this database, when the only improvement is to hide some outdated messages :/

@madebr
Copy link

madebr commented Nov 17, 2020

I just got 14+ mails from your bot which is a bit annoying.
The database should, imho, only contain the following:

  • repo: conan-io/conan-center-index
  • issue number: 2867
  • comment: 729036085

Then it knows what message to mutilate.
You can simply store it as a json/yml.

Or you could iterate through all comments on an issue and look for messages made by your bot.
Then no state should be kept.

@ericLemanissier
Copy link
Owner Author

Sorry about the round of emails 40 minutes ago, it was an error from me. It should not occur again. I changed the bot to not modify posts so that it does not generate unnecessary notifications each time the CI runs

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

No branches or pull requests

3 participants