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

Feat(Comment): Add new parameters commentSuccessTo #678

Closed
wants to merge 1 commit into from

Conversation

Hakihiro
Copy link

Choose between MR or Issue message to be posted. Both is also allowed

Close: #636

Choose between MR or Issue message to be posted. Both is also allowed
@fgreinacher
Copy link
Contributor

fgreinacher commented Feb 19, 2024

Thanks for the PR @Hakihiro, greatly appreciated!

As mentioned in #480 (comment) I would prefer to solve this without a new configuration option.

Otherwise we might end with many similar options like commentSuccessForBranches, commentSuccessForStableVersions and in the worst case we'd need combinations of them 😬

@JonasSchubert
Copy link
Contributor

Thanks for the PR @Hakihiro, greatly appreciated!

As mentioned in #480 (comment) I would prefer to solve this without a new configuration option.

Otherwise we might end with many similar options like commentSuccessForBranches, commentSuccessForStableVersions and in the worst case we'd need combinations of them 😬

Hi @Hakihiro and thanks a lot for your contribution. I agree with @fgreinacher the risk of configuration overhead is too big and trying a templating attempt might be more sustainable.

@Hakihiro
Copy link
Author

I understand your point of view. I'm not a big fan to put an information inside a template message to determine on which target(s) it should be posted. But I understand your fear about the unsustainable configuration becoming quickly unmainteable.

I will try to extract this information from the successMessage with lodash template, base on the following syntax:
[to issues] - [to merge-request] - [to issues,merge-request]

Is this something more suitable for you?

Thanks for your feedback

@fgreinacher
Copy link
Contributor

Thanks a ton!

I will try to extract this information from the successMessage with lodash template, base on the following syntax:
[to issues] - [to merge-request] - [to issues,merge-request]

That's not even needed. The idea would be to not create the note if the rendered template returns a falsey value:

https://github.com/semantic-release/gitlab/blob/master/lib/success.js#L31-L38

Users could then define a template like this to indicate that they only want comments on issues (not sure about the exact syntax, needs some experimentation with the lodash template engine:

<% if (issue) { %> regular comment <% } %>

@JonasSchubert
Copy link
Contributor

Hi @Hakihiro and sorry already as I cross you a bit with another pull request. There was no activity here for some time now, so I created a new PR with these suggestions.

@fgreinacher
Copy link
Contributor

Closing in favor of #730.

@fgreinacher fgreinacher closed this Jun 5, 2024
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.

Possibility to send succesMessage only for Issue
3 participants