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

[GH #175] add autocomplete for gitlab slash command #182

Merged
merged 11 commits into from
Jul 23, 2020
Merged

[GH #175] add autocomplete for gitlab slash command #182

merged 11 commits into from
Jul 23, 2020

Conversation

ericjaystevens
Copy link
Contributor

add autocomplete for gitlab slash command issue# 175

Summary

Adds autocomplete support for slash command

Ticket Link

Fixes #175

add autocomplete for gitlab slash command issue# 175
@codecov
Copy link

codecov bot commented Jun 14, 2020

Codecov Report

Merging #182 into master will decrease coverage by 1.47%.
The diff coverage is 15.45%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #182      +/-   ##
==========================================
- Coverage   42.31%   40.84%   -1.48%     
==========================================
  Files          16       16              
  Lines        1633     1714      +81     
==========================================
+ Hits          691      700       +9     
- Misses        871      944      +73     
+ Partials       71       70       -1     
Impacted Files Coverage Δ
server/command.go 33.78% <15.45%> (-6.43%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e5d1e89...e1f2867. Read the comment docs.

@iomodo iomodo added the 2: Dev Review Requires review by a core committer label Jun 15, 2020
@iomodo iomodo added this to the v1.2.0 milestone Jun 15, 2020
@iomodo iomodo requested a review from hanzei June 15, 2020 06:38
Copy link
Contributor

@iomodo iomodo left a comment

Choose a reason for hiding this comment

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

Great! Thanks @ericjaystevens !
Couple of comments bellow.

server/command.go Outdated Show resolved Hide resolved
server/command.go Show resolved Hide resolved
server/command.go Show resolved Hide resolved
@iomodo iomodo added the 1: PM Review Requires review by a product manager label Jun 15, 2020
Copy link
Collaborator

@hanzei hanzei left a comment

Choose a reason for hiding this comment

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

This will be a quite handy improvement for UX

go.mod Outdated Show resolved Hide resolved
server/command.go Outdated Show resolved Hide resolved
server/command.go Outdated Show resolved Hide resolved
server/command.go Outdated Show resolved Hide resolved
server/command.go Outdated Show resolved Hide resolved
server/command.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@hanzei hanzei left a comment

Choose a reason for hiding this comment

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

Great work 🎉 Can't wait to see these in action.

Two thoughts on the naming

cc @aaronrothschild @iomodo

server/command.go Outdated Show resolved Hide resolved
server/command.go Outdated Show resolved Hide resolved
@ericjaystevens
Copy link
Contributor Author

Seem's like the consensus is to change /gitlab unsubscribe and /gitlab subscribe [subcommand] to /gitlab subscriptions [deleted | add | list ] with aliases for /gitlab unsubscribe and /gitlab subscribe [subcommand] I'll move forward with that goal.

We can generalize this that a convention exists for slash subcommands of /slashcommand noun verb arguments

Copy link
Collaborator

@hanzei hanzei left a comment

Choose a reason for hiding this comment

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

Awesome work 🎉 Just one missing command in the help text

server/command.go Outdated Show resolved Hide resolved
server/command.go Outdated Show resolved Hide resolved
Copy link
Contributor

@iomodo iomodo left a comment

Choose a reason for hiding this comment

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

Thanks @ericjaystevens looks great to me! Awesome!

Copy link
Collaborator

@hanzei hanzei left a comment

Choose a reason for hiding this comment

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

Great work @ericjaystevens 🎉 This is awesome

@hanzei hanzei added 3: QA Review Requires review by a QA tester and removed 2: Dev Review Requires review by a core committer labels Jul 20, 2020
@hanzei hanzei requested a review from DHaussermann July 20, 2020 12:51
Copy link

@DHaussermann DHaussermann left a comment

Choose a reason for hiding this comment

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

Tested and passed

  • Tested that all commands are successfully added to auto-complete
  • Smoke tested the plugin using auto complete functionality
  • Tested on desktop and browser
  • No issues found
    LGTM!

Huge thanks @ericjaystevens for implementing this! This is especially nice to use with subscriptions and webhooks functionality.

@DHaussermann DHaussermann added 4: Reviews Complete All reviewers have approved the pull request and removed 1: PM Review Requires review by a product manager 3: QA Review Requires review by a QA tester labels Jul 23, 2020
@mickmister mickmister merged commit 3644757 into mattermost:master Jul 23, 2020
@iomodo iomodo mentioned this pull request Jul 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add slash command autocomplete functionality
7 participants