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

Possibility to send succesMessage only for Issue #636

Open
Hakihiro opened this issue Nov 22, 2023 · 5 comments · May be fixed by #678
Open

Possibility to send succesMessage only for Issue #636

Hakihiro opened this issue Nov 22, 2023 · 5 comments · May be fixed by #678
Labels

Comments

@Hakihiro
Copy link

Hi, first, thanks for your work on this project.

Second, I'm guessing if there is any possibility to disable success message but only for issue or MR ?

Because right now it's quite redundant to have the message in the issue and MR. And we can only disable success message for both, but not only for one of them.

If not, is this something you will be open to have in the project.

Sorry if this message is silly, I could not find any information about it.

@JonasSchubert
Copy link
Contributor

JonasSchubert commented Nov 22, 2023

IMO this is a valid request. This could also reduce the API requests, which was either an issue or a similar feature request also here, here and here.

Similar to @nejch 's suggestion, we could introduce properties commentSuccessTo and commentFailureTo (just initial idea with being open for other property names) with defaults (also if not set to avoid breaking changes) for merge-request,issue and allow to either chose merge-request or issue only.

@Hakihiro
Copy link
Author

Hi, thanks for your response.

Your proposal seems to be (in my case), what I need.

I don't see this as something complicated either for the utilization or the implementation, are you open to PR or should we wait the approval of the team about it?

@JonasSchubert
Copy link
Contributor

Hey @Hakihiro , personally, I would wait for team response before starting to work on that.
There have been some discussions related to similar topics and maybe we can bundle it somehow.

But doesn't mean you shouldn't start working on that or anybody else 😉

@fgreinacher
Copy link
Contributor

fgreinacher commented Nov 27, 2023

Thanks for reaching out @Hakihiro!

I still think we should first explore the suggestion by @olexs at #480 (comment) as this would be most flexible while keeping the configuration surface small.

Feel free to send a PR for this so that we can see how that would feel.

@Hakihiro
Copy link
Author

Hi, sorry for the delay.

I made it quickly (really inspired by @JonasSchubert suggestion, thanks btw) just to validate the global design. If it's validate by the team, I can add tests and take code feedbacks to improve the modifications, and the documentation ofc.

I think it could be a first step, and it could be overridden later by an information extracted from the comment template.
This way is really the more natural for the user to have an explicit way to configure it, and it can really be easier to understand from a documentation.

I'm open to discuss it.

Thanks again for your time

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

Successfully merging a pull request may close this issue.

3 participants