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

Notify GitLab Plugin: add shortDescription option #558

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Breuls
Copy link

@Breuls Breuls commented Nov 26, 2021

What does this change?

It adds the option shortDescription to the Notify GitLab Plugin. I took the liberty of copying the implementation from the GitHub 'with API' plugin, which already did exactly what I was looking for.

I used a new config parameter for the plugin and then passed it through from the GitLabNotifierPlugin class to the functions in use-cases.ts and then to the config object for the comment function. This adds the name of the new parameter in a lot of places in those files, which may look a bit messy. However, the only alternative I could think of was adding it to the notifyParams. Doing that potentially impacts code in other places, which I thought was a bit overkill.

Let me know if you'd rather see it added to notifyParams.

In addition to the changes, I added tests to the files that I changed, covering all lines, branches, et cetera.

References

N/A

Screenshots

N/A

What can I check for bug fixes?

N/A - No bugs.

The functional change is: set shortDescription to true. The comment posted to GitLab will then be a short table summary instead of a sea of coloured icons.

@Breuls
Copy link
Author

Breuls commented Nov 29, 2021

I added another commit: I discovered that because the code in create-comment.ts was indented, the table Markdown in the comment was also indented and that caused GitLab to interpret those lines as "code", which means that it was printed in a code block instead of converted to HTML.

The added commit adds a replace call to the output, plus a test to confirm that it works correctly.

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.

1 participant