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-593] Add feature flag for new pull requests #712

Merged
merged 7 commits into from
Dec 4, 2023
Merged

[GH-593] Add feature flag for new pull requests #712

merged 7 commits into from
Dec 4, 2023

Conversation

San4es
Copy link
Contributor

@San4es San4es commented Nov 16, 2023

This is recreation of @C-Deck's PR #612 with additional changes addressing these code review comments:
#612 (comment)

Closes #593.

@San4es San4es requested a review from hanzei as a code owner November 16, 2023 15:48
@mattermost-build
Copy link
Contributor

Hello @San4es,

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.

Per the Mattermost Contribution Guide, we need to add you to the list of approved contributors for the Mattermost project.

Please help complete the Mattermost contribution license agreement?
Once you have signed the CLA, please comment with /check-cla and confirm that the CLA check is green.

This is a standard procedure for many open source projects.

Please let us know if you have any questions.

We are very happy to have you join our growing community! If you're not yet a member, please consider joining our Contributors community channel to meet other contributors and discuss new opportunities with the core team.

@San4es
Copy link
Contributor Author

San4es commented Nov 16, 2023

/check-cla

@hanzei hanzei added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Nov 16, 2023
Copy link
Contributor

@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.

Nice, thanks wrapping up this afford 🚀

Copy link
Contributor

@mickmister mickmister left a comment

Choose a reason for hiding this comment

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

Thanks @San4es! LGTM, just one non-blocking suggestion to improve the test

server/plugin/command_test.go Outdated Show resolved Hide resolved
@mickmister mickmister removed the 2: Dev Review Requires review by a core committer label Nov 20, 2023
@hanzei hanzei added this to the v2.2.0 milestone Nov 20, 2023
@codecov-commenter
Copy link

codecov-commenter commented Nov 24, 2023

Codecov Report

Attention: 14 lines in your changes are missing coverage. Please review.

Comparison is base (8cf783f) 15.49% compared to head (9fa9a04) 15.76%.

❗ Current head 9fa9a04 differs from pull request most recent head 12a2aa5. Consider uploading reports for the commit 12a2aa5 to get more accurate results

Files Patch % Lines
server/plugin/command.go 55.00% 9 Missing ⚠️
server/plugin/webhook.go 0.00% 3 Missing ⚠️
server/plugin/subscriptions.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #712      +/-   ##
==========================================
+ Coverage   15.49%   15.76%   +0.26%     
==========================================
  Files          15       15              
  Lines        5737     5755      +18     
==========================================
+ Hits          889      907      +18     
  Misses       4806     4806              
  Partials       42       42              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hanzei
Copy link
Contributor

hanzei commented Nov 27, 2023

@AayushChaudhary0001 The PR is ready for your review

Copy link
Contributor

@mickmister mickmister left a comment

Choose a reason for hiding this comment

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

Thanks @San4es!

Clint Decker and others added 6 commits November 28, 2023 20:07
- Add new_pulls feature flag
- Add check for the flag in postPullRequestEvent
- Add check for conflicting flags and refactor the conflicting checks
to a helper function
@hanzei hanzei mentioned this pull request Nov 30, 2023
Copy link

@AayushChaudhary0001 AayushChaudhary0001 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 approved, LGTM.

@hanzei hanzei added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester labels Dec 4, 2023
@hanzei hanzei merged commit f5d2291 into mattermost:master Dec 4, 2023
9 checks passed
@San4es San4es deleted the feature/NewPulls branch December 5, 2023 18:31
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 Contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Just posting new pull requests
6 participants