Closed sibasankarnayak closed 2 years ago
Hello @sibasankarnayak,
Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.
Merging #66 (e23774d) into master (c734bcf) will decrease coverage by
0.77%
. The diff coverage is6.42%
.
@@ Coverage Diff @@
## master #66 +/- ##
==========================================
- Coverage 16.98% 16.20% -0.78%
==========================================
Files 13 13
Lines 1772 1851 +79
==========================================
- Hits 301 300 -1
- Misses 1453 1533 +80
Partials 18 18
Impacted Files | Coverage Δ | |
---|---|---|
server/command.go | 6.79% <0.00%> (-2.94%) |
:arrow_down: |
server/plugin.go | 3.23% <100.00%> (-0.29%) |
:arrow_down: |
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 c734bcf...e23774d. Read the comment docs.
@mickmister @hanzei added this two feature in this PR too https://github.com/mattermost/mattermost-plugin-github/issues/513 https://github.com/mattermost/mattermost-plugin-github/issues/512
I still think the code in https://github.com/mattermost/mattermost-plugin-github/blob/4db6b6a461d7d1ec5e137ee8a744b6ced3ed4e2d/server/plugin/command.go#L224-L242 is cleaner, but this is also fine.
@hanzei should i make it clean ?
Your call. I'm fine with the current code.
Your call. I'm fine with the current code.
would perfer to keep it
@sibasankarnayak Please take a look at @mickmister and let me know what you think
@sibasankarnayak pls ping me when everything is done from your side so i can test it.
@sibasankarnayak pls ping me when everything is done from your side so i can test it.
@dipak-demansol Please test this and share your feedback
@dipak-demansol Gentle reminder to review this PR
@dipak-demansol Gentle reminder to review this PR
Testing is done, There are few point that i need to discuss with dylan so end of the day i'll add my comment.
@sibasankarnayak , can you address @dipak-demansol 's comments? We were targeting the release of this plugin for last month. Thanks.
@sibasankarnayak , can you address @dipak-demansol 's comments? We were targeting the release of this plugin for last month. Thanks.
@catalintomai sure @dipak-demansol can you check now.
@sibasankarnayak , can you address @dipak-demansol 's comments? We were targeting the release of this plugin for last month. Thanks.
@catalintomai sure @dipak-demansol can you check now.
@sibasankarnayak 1) When i Use "//bitbucket subscriptions" AND add any parameter ex "/bitbucket subscriptions ABCD" then its add "ABCD" but i did not used the "add" Command, The user should get the validation message.
2) With the list Command if i pass any extra parameter then its behaving as "add" Functionality.
- /bitbucket subscriptions ABCD
@dipak-demansol thank you for finding such cases, can you test now
added slash command for plugin
ticket here Fixes #51