Disable merging a WIP Pull request #4529

JulienTant · 2018-07-27T22:24:32Z

This PR fixes #4453

The idea behind this pull request is that when the TITLE of the pull requests starts with [wip] or wip:, then the nobody can merge that pull request until the "wip" indicator is removed from the title

Disable the ability to merge when the PR contains the given prefixes in the UI
Disable the ability to merge when the PR contains the given prefixes in the API
Write tests
Find a way to inject settings in js
Add documentation

codecov-io · 2018-07-27T22:31:39Z

Codecov Report

Merging #4529 into master will increase coverage by 0.01%.
The diff coverage is 34.28%.

@@            Coverage Diff             @@
##           master    #4529      +/-   ##
==========================================
+ Coverage   20.63%   20.65%   +0.01%     
==========================================
  Files         166      166              
  Lines       32262    32295      +33     
==========================================
+ Hits         6657     6669      +12     
- Misses      24627    24646      +19     
- Partials      978      980       +2

Impacted Files	Coverage Δ
routers/repo/issue.go	`0.91% <0%> (-0.01%)`	⬇️
routers/api/v1/repo/pull.go	`0% <0%> (ø)`	⬆️
routers/repo/pull.go	`0% <0%> (ø)`	⬆️
models/pull.go	`17.65% <57.14%> (+0.91%)`	⬆️

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 52c2cb1...a2bf0bb. Read the comment docs.

lafriks · 2018-07-27T22:46:22Z

options/locale/locale_en-US.ini

 pulls.data_broken = This pull request is broken due to missing fork information.
 pulls.is_checking = "Merge conflict checking is in progress. Try again in few moments."
 pulls.can_auto_merge_desc = This pull request can be merged automatically.
 pulls.cannot_auto_merge_desc = This pull request cannot be merged automatically due to conflicts.
 pulls.cannot_auto_merge_helper = Merge manually to resolve the conflicts.
 pulls.no_merge_desc = This pull request cannot be merged because all repository merge options are disabled.
 pulls.no_merge_helper = Enable merge options in the repository settings or merge the pull request manually.
+pulls.no_merge_wip = "This pull request cannot be merged because it is marked as being a work in progress."


shoud be can not

lafriks · 2018-07-27T22:47:29Z

options/locale/locale_en-US.ini

@@ -829,13 +829,15 @@ pulls.tab_files = Files Changed
 pulls.reopen_to_merge = Please reopen this pull request to perform a merge.
 pulls.merged = Merged
 pulls.has_merged = The pull request has been merged.
+pulls.work_in_progress = "This is a work in progress. Remove the <strong>%s</strong> prefix from the title when it's ready"


This is a work in progress. would better be This Pull Request is marked as a work in progress.

lafriks · 2018-07-27T23:54:03Z

options/locale/locale_en-US.ini

@@ -829,13 +829,15 @@ pulls.tab_files = Files Changed
 pulls.reopen_to_merge = Please reopen this pull request to perform a merge.
 pulls.merged = Merged
 pulls.has_merged = The pull request has been merged.
+pulls.work_in_progress = "This Pull Request is marked as a work in progress. Remove the <strong>%s</strong> prefix from the title when it's ready"


sorry my mistake. Pull Request should be pull request like in other messages

lafriks · 2018-07-28T00:22:44Z

integrations/pull_merge_test.go

+
+	resp := testPullCreate(t, session, "user1", "repo1", "master", "[wip] This is a pull title")
+
+	elem := strings.Split(test.RedirectURL(resp), "/")


I don't think this check is needed

JulienTant · 2018-07-29T22:43:20Z

I added some tests + a helper under the PR title input to add WIP: in one click :-)

adelowo · 2018-07-30T11:28:44Z

integrations/pull_merge_test.go

@@ -13,6 +13,7 @@ import (

 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/test"
+	"github.com/Unknwon/i18n"


Shouldn't this come after the blank line ?

JulienTant · 2018-07-30T14:10:38Z

Do you guys can help me about the documentation ? I can't decide where to put a paragraph on that feature. Should I document it ?

JulienTant · 2018-07-31T22:08:32Z

Dealing with "Work In Progress" pull requests

Marking a pull request as being a work in progress will prevent that pull request from being accidentally merged. To mark a pull request as being a work in progress, you must prefix its title by WIP: or [WIP] (case insensitive)

lunny · 2018-08-01T03:02:59Z

models/pull.go

@@ -1195,6 +1195,43 @@ func (pr *PullRequest) checkAndUpdateStatus() {
 	}
 }

+// Enumerate the prefixes that will help determine if a pull request is a Work In Progress
+var pullRequestWorkInProgressPrefixes = [...]string{


I thinik it's better to set these as default and put them to app.ini files.

lunny · 2018-08-01T03:05:16Z

@JulienTant I think you can add a new page under usage named pull request and put a section there.

Signed-off-by: Julien Tant <julien@craftyx.fr>

…in templates

JulienTant · 2018-08-01T20:23:17Z

Ready for review 💪

adelowo · 2018-08-02T06:17:46Z

docs/content/doc/usage/pull-request.en-us.md

+
+## Pull Request Templates
+
+You can find more information about pull request templates into the dedicated page : [Issue and Pull Request templates](../issue-pull-request-templates)


I think this should be in the dedicated page not into the dedicated page

adelowo · 2018-08-02T06:20:59Z

docs/content/doc/usage/pull-request.en-us.md

+
+## "Work In Progress" pull requests
+
+Marking a pull request as being a work in progress will prevent that pull request from being accidentally merged. To mark a pull request as being a work in progress, you must prefix its title by `WIP:` or `[WIP]` (case insensitive). Those values are configurable in your `app.ini` file :


it’s title

i'm pretty sure that its is correct, http://grammarist.com/spelling/its-its/

Oops... Thanks for the link

lafriks · 2018-08-09T06:25:48Z

public/js/index.js

+        var $issueTitle = $("#issue_title");
+        var value = $issueTitle.val().trim().toUpperCase();
+
+        var addPrefix = true;


There is no need for this variable. Can be changed so:

for (var i in wipPrefixes) { if (value.startsWith(wipPrefixes[i].toUpperCase())) { return; } } $issueTitle.val(wipPrefixes[0] + " " + $issueTitle.val());

lafriks · 2018-08-09T06:30:38Z

integrations/api_pull_test.go

+	prepareTestEnv(t)
+	repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository)
+	owner := models.AssertExistsAndLoadBean(t, &models.User{ID: repo.OwnerID}).(*models.User)
+	pr := models.AssertExistsAndLoadBean(t, &models.PullRequest{Status: models.PullRequestStatusMergeable, HasMerged: false}).(*models.PullRequest)


Does this PR really have WIP prefix?

Wow this is a mistake i'm sorry :/

lunny · 2018-08-13T01:58:56Z

LGTM

lafriks

Except for translation file quoting otherwise LG-TM

lafriks · 2018-08-13T15:56:50Z

options/locale/locale_en-US.ini

 pulls.data_broken = This pull request is broken due to missing fork information.
 pulls.is_checking = "Merge conflict checking is in progress. Try again in few moments."
 pulls.can_auto_merge_desc = This pull request can be merged automatically.
 pulls.cannot_auto_merge_desc = This pull request cannot be merged automatically due to conflicts.
 pulls.cannot_auto_merge_helper = Merge manually to resolve the conflicts.
 pulls.no_merge_desc = This pull request cannot be merged because all repository merge options are disabled.
 pulls.no_merge_helper = Enable merge options in the repository settings or merge the pull request manually.
+pulls.no_merge_wip = "This pull request can not be merged because it is marked as being a work in progress."


No need for quoting with "

lafriks · 2018-08-13T15:58:21Z

options/locale/locale_en-US.ini

@@ -841,13 +841,16 @@ pulls.tab_files = Files Changed
 pulls.reopen_to_merge = Please reopen this pull request to perform a merge.
 pulls.merged = Merged
 pulls.has_merged = The pull request has been merged.
+pulls.title_wip_desc = '<a href="#">Start the title with <strong>%s</strong></a> to prevent the pull request from being merged accidentally.'


There would be needed backtick (`) quotes

lafriks · 2018-08-13T16:00:46Z

options/locale/locale_en-US.ini

@@ -841,13 +841,16 @@ pulls.tab_files = Files Changed
 pulls.reopen_to_merge = Please reopen this pull request to perform a merge.
 pulls.merged = Merged
 pulls.has_merged = The pull request has been merged.
+pulls.title_wip_desc = '<a href="#">Start the title with <strong>%s</strong></a> to prevent the pull request from being merged accidentally.'
+pulls.cannot_merge_work_in_progress = "This pull request is marked as a work in progress. Remove the <strong>%s</strong> prefix from the title when it's ready"


No need for quoting with "

JulienTant · 2018-08-13T18:40:05Z

I decided to try everything one last time & found a bug, that I fixed: the link bellow the title bar (see bellow) was not working because of a refactoring I made on names :/ This is now fix!

I also remove the useless quote as requested by @lafriks

lafriks

Thanks, great work!

Prepare for wild/v1.6 branch * BREAKING * Respect email privacy option in user search via API (go-gitea#4512) * Simply remove tidb and deps (go-gitea#3993) * Swagger.v1.json template (go-gitea#3572) * FEATURE * Pull request review/approval and comment on code (go-gitea#3748) * Added dependencies for issues (go-gitea#2196) (go-gitea#2531) * Add the ability to have built in themes in Gitea and provide dark theme arc-green (go-gitea#4198) * Add sudo functionality to the API (go-gitea#4809) * Add oauth providers via cli (go-gitea#4591) * Disable merging a WIP Pull request (go-gitea#4529) * Force user to change password (go-gitea#4489) * Add letsencrypt to Gitea (go-gitea#4189) * Add push webhook support for mirrored repositories (go-gitea#4127) * Add csv file render support defaultly (go-gitea#4105) * Add Recaptcha functionality to Gitea (go-gitea#4044) * BUGFIXES * Fix release creation via API (go-gitea#5076) * Remove links from topics in edit mode (go-gitea#5026) * Fix missing AppSubUrl in few more templates (fixup) (go-gitea#5021) * Fix missing AppSubUrl in some templates (go-gitea#5020) * Hide outdated comments in file view (go-gitea#5017) * Upgrade gopkg.in/testfixtures.v2 (go-gitea#4999) * Disable debug routes unless PPROF is enabled in configuration (go-gitea#4995) * Fix user menu item styling (go-gitea#4985) * Fix layout of the topics editing form (go-gitea#4971) * Fix null pointer dereference in ParseCommitWithSignature (go-gitea#4962) * Fix url in discord webhook (go-gitea#4953) * Detect charset and convert non UTF-8 files for display (go-gitea#4950) * Make sure to catch the right error so it is displayed on the UI (go-gitea#4945) * Fix(topics): don't redirect to explore page. (go-gitea#4938) * Fix bug forget to remove Stopwatch when remove repository (go-gitea#4928) * Fix bug when repo remained bare if multiple branches pushed in single push (go-gitea#4923) * Fix: Let's Encrypt configuration settings (go-gitea#4911) * Fix: Crippled diff (go-gitea#4726) (go-gitea#4900) * Fix trimming of markup section names (go-gitea#4863) * Issues api allow pulls and fix go-gitea#4832 (go-gitea#4852) * Do not autocreate directory for new users/orgs (go-gitea#4828) (go-gitea#4849) * Fix redirect with non-ascii branch names (go-gitea#4764) (go-gitea#4810) * Fix missing release title in webhook (go-gitea#4783) (go-gitea#4796) * User shouldn't be able to approve or reject his/her own PR (go-gitea#4729) * Make sure to reset commit count in the cache on mirror syncing (go-gitea#4720) * Fixed bug where team with admin privelege type doesn't get any unit (go-gitea#4719) * Fix incorrect caption of webhook setting (go-gitea#4701) (go-gitea#4717) * Allow WIP marker to contains < or > (go-gitea#4709) * Hide org/create menu item in Dashboard if user has no rights (go-gitea#4678) (go-gitea#4680) * Site admin could create repos even MAX_CREATION_LIMIT=0 (go-gitea#4645) * Fix custom templates being ignored (go-gitea#4638) * Fix starring icon after semantic ui update (go-gitea#4628) * Fix Split-View line adjustment (go-gitea#4622) * Fix integer constant overflows in tests (go-gitea#4616) * Push whitelist now doesn't apply to branch deletion (go-gitea#4601) (go-gitea#4607) * Fix bugs when too many IN variables (go-gitea#4594) * Fix failure on creating pull request with assignees (go-gitea#4419) (go-gitea#4583) * Fix panic issue on update avatar email (go-gitea#4580) (go-gitea#4581) * Fix status code label for a successful webhook (go-gitea#4540) * An inactive user shouldn't be able to be added as a collaborator (go-gitea#4535) * Don't fail silently if trying to add a collaborator twice (go-gitea#4533) * Fix incorrect MergeWhitelistTeamIDs check in CanUserMerge function (go-gitea#4519) (go-gitea#4525) * Fix out-of-transaction query in removeOrgUser (go-gitea#4521) (go-gitea#4522) * Fix migration from older releases (go-gitea#4495) * Accept 'Data:' in commit graph (go-gitea#4487) * Update xorm to latest version and fix correct `user` table referencing in sql (go-gitea#4473) * Relative URLs for LibreJS page (go-gitea#4460) * Redirect to correct page after using scratch token (go-gitea#4458) * Fix column droping for MSSQL that need new transaction for that (go-gitea#4440) * Replace src with raw to fix image paths (go-gitea#4377) * Add default merge options when creating new repository (go-gitea#4369) * Fix docker build (go-gitea#4358) * Fixes repo membership check in API (go-gitea#4341) * Dep upgrade mysql lib (go-gitea#4161) * Fix some issues with special chars in branch names (go-gitea#3767) * Responsive design fixes (go-gitea#4508) * ENHANCEMENT * Fix milestones sorted wrongly (go-gitea#4987) * Allow api to create tags for releases if they don't exist (go-gitea#4890) * Fix go-gitea#4877 to follow the OpenID Connect Audiences spec (go-gitea#4878) * Enforce token on api routes [fixed critical security issue go-gitea#4357] (go-gitea#4840) * Update legacy branch and tag URLs in dashboard to new format (go-gitea#4812) * Slack webhook channel name cannot be empty or just contain an hashtag (go-gitea#4786) * Add whitespace handling to PR-comparsion (go-gitea#4683) * Make reverse proxy auth optional (go-gitea#4643) * MySQL TLS (go-gitea#4642) * Make sure to set PR split view when creating/previewing a pull request (go-gitea#4617) * Log user in after a successful sign up (go-gitea#4615) * Fix typo IsPullReuqestBroken -> IsPullRequestBroken (go-gitea#4578) * Allow admin toggle forcing a password change for newly created users (go-gitea#4563) * Update jQuery to v1.12.4 (go-gitea#4551) * Env var GITEA_PUSHER_EMAIL (go-gitea#4516) * Feat(repo): support search repository by topic name (go-gitea#4505) * Small improvements to dependency UI (go-gitea#4503) * Make max commits in graph configurable (go-gitea#4498) * Add valid for lfs oid (go-gitea#4461) * Add shortcut to save wiki page (go-gitea#4452) * Allow administrator to create repository for any organization (go-gitea#4368) * Fix repository last updated time update when delete a user who watched the repo (go-gitea#4363) * Switch plaintext scratch tokens to use hash instead (go-gitea#4331) * Increase default TOTP secret size to 320 bits (go-gitea#4287) * Keep preseeded database password (go-gitea#4284) * Implemented hover text showing user FullName (go-gitea#4261) * Add ability to delete a token (go-gitea#4235) * Fix typos in i18n variable names. (go-gitea#4080) * Api: repos/search: add parameters to control the sort order (go-gitea#3964) * Add missing path in the Docker app.ini template (go-gitea#2181) * Add file name and branch to page title (go-gitea#4902) * Offline use of google fonts (go-gitea#4872) * Add missing History link to directory listings v2 (go-gitea#4829) * Locale for Edit and Remove due date issue (go-gitea#4802) * Disable 'May Import Local Repository' when is disabled by setting (Is… (go-gitea#4780) * API /admin/users/{username} missing parameter (go-gitea#4775) * Display error when adding a user to a team twice (go-gitea#4746) * Remove UsePrivilegeSeparation from the Docker sshd_config, see go-gitea#2876 (go-gitea#4722) * Focus title input when clicking helper link (go-gitea#4696) * Add vendor to user reserved words and format words list according alphabet (go-gitea#4685) * Add gitea/issues link to 500 page (go-gitea#4654) * Hide home button when landing page is not set to home (go-gitea#4651) * Remove link to GitHub issues in 404 template (go-gitea#4639) * Cmd/serve: pprof cpu and memory profile dumps to disk (go-gitea#4560) * Add flash message after an account has been successfully activated (go-gitea#4510) * Prevent html entity escaping on delete branch (go-gitea#4471) * Locale for button Edit on protected branch (go-gitea#4442) * Update notification icon (go-gitea#4343) * Added front-end topics validation (go-gitea#4316) * Don't display buttons if there are no system notifications (go-gitea#4280) * Issue due date api (go-gitea#3890) * SECURITY * Improve URL validation for external wiki and external issues (go-gitea#4710) * Make cookies HttpOnly and obey COOKIE_SECURE flag (go-gitea#4706) * Don't disclose emails of all users when sending out emails (go-gitea#4664) * Check that repositories can only be migrated to own user or organizations (go-gitea#4366) * TRANSLATION * Fix punctuation in English translation (go-gitea#4958) * Fix translation (go-gitea#4355)

bkcsoft added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 27, 2018

lafriks added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Jul 27, 2018

lafriks added this to the 1.6.0 milestone Jul 27, 2018

lafriks reviewed Jul 27, 2018

View reviewed changes

JulienTant force-pushed the add-wip-control-on-pr branch from 2827a1e to f1d936a Compare July 27, 2018 23:08

lafriks reviewed Jul 27, 2018

View reviewed changes

lafriks reviewed Jul 28, 2018

View reviewed changes

JulienTant force-pushed the add-wip-control-on-pr branch 2 times, most recently from e16aefd to 0888785 Compare July 29, 2018 21:35

adelowo reviewed Jul 30, 2018

View reviewed changes

lunny reviewed Aug 1, 2018

View reviewed changes

JulienTant added 5 commits August 1, 2018 09:34

prevent pull request to be merged when PR is a WIP

b51d021

Signed-off-by: Julien Tant <julien@craftyx.fr>

add tests

4e04e18

Signed-off-by: Julien Tant <julien@craftyx.fr>

add helper to prepend WIP: in PR title

2950078

regroup external imports

864f795

Signed-off-by: Julien Tant <julien@craftyx.fr>

move default wip prefixes into settings

195b413

Signed-off-by: Julien Tant <julien@craftyx.fr>

JulienTant force-pushed the add-wip-control-on-pr branch from 1c41fc8 to 195b413 Compare August 1, 2018 07:34

JulienTant added 2 commits August 1, 2018 18:54

use configurable WIP prefixes in javascript and default to first one …

eaadddd

…in templates

add documentation

f150a20

JulienTant force-pushed the add-wip-control-on-pr branch from ffbb263 to f150a20 Compare August 1, 2018 19:48

add unit test on pull model

45732db

Merge branch 'master' into add-wip-control-on-pr

f146aea

adelowo reviewed Aug 2, 2018

View reviewed changes

JulienTant added 2 commits August 2, 2018 12:15

typo

404541a

Merge branch 'master' into add-wip-control-on-pr

af9f432

lafriks reviewed Aug 9, 2018

View reviewed changes

JulienTant added 2 commits August 12, 2018 23:00

simplify javascript

6f766a2

fix api test

7dbb842

bkcsoft added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Aug 13, 2018

lafriks reviewed Aug 13, 2018

View reviewed changes

fix js+remove useless quotes in documentation

cdecb10

lafriks approved these changes Aug 13, 2018

View reviewed changes

bkcsoft added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Aug 13, 2018

Merge branch 'master' into add-wip-control-on-pr

a2bf0bb

lafriks merged commit 7781e8c into go-gitea:master Aug 13, 2018

JulienTant deleted the add-wip-control-on-pr branch August 13, 2018 19:06

InExtremaRes mentioned this pull request Aug 14, 2018

If the first word at WORK_IN_PROGRESS_PREFIXES starts with <, the hint is shown incorrectly #4705

Closed

lafriks added the type/changelog Adds the changelog for a new Gitea version label Nov 10, 2018

go-gitea locked and limited conversation to collaborators Nov 24, 2020

delvh removed the type/changelog Adds the changelog for a new Gitea version label Oct 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disable merging a WIP Pull request #4529

Disable merging a WIP Pull request #4529

JulienTant commented Jul 27, 2018 •

edited

Loading

codecov-io commented Jul 27, 2018 •

edited

Loading

lafriks Jul 27, 2018

lafriks Jul 27, 2018

lafriks Jul 27, 2018

lafriks Jul 28, 2018

JulienTant commented Jul 29, 2018

adelowo Jul 30, 2018 •

edited

Loading

JulienTant commented Jul 30, 2018 •

edited

Loading

JulienTant commented Jul 31, 2018

lunny Aug 1, 2018

lunny commented Aug 1, 2018

JulienTant commented Aug 1, 2018

adelowo Aug 2, 2018

adelowo Aug 2, 2018 •

edited

Loading

JulienTant Aug 2, 2018

adelowo Aug 2, 2018

lafriks Aug 9, 2018

lafriks Aug 9, 2018

JulienTant Aug 12, 2018

lunny commented Aug 13, 2018

lafriks left a comment

lafriks Aug 13, 2018

lafriks Aug 13, 2018

lafriks Aug 13, 2018

JulienTant commented Aug 13, 2018

lafriks left a comment


		resp := testPullCreate(t, session, "user1", "repo1", "master", "[wip] This is a pull title")

		elem := strings.Split(test.RedirectURL(resp), "/")


		## Pull Request Templates

		You can find more information about pull request templates into the dedicated page : [Issue and Pull Request templates](../issue-pull-request-templates)


		## "Work In Progress" pull requests

		Marking a pull request as being a work in progress will prevent that pull request from being accidentally merged. To mark a pull request as being a work in progress, you must prefix its title by `WIP:` or `[WIP]` (case insensitive). Those values are configurable in your `app.ini` file :

Disable merging a WIP Pull request #4529

Disable merging a WIP Pull request #4529

Conversation

JulienTant commented Jul 27, 2018 • edited Loading

codecov-io commented Jul 27, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JulienTant commented Jul 29, 2018

adelowo Jul 30, 2018 • edited Loading

Choose a reason for hiding this comment

JulienTant commented Jul 30, 2018 • edited Loading

JulienTant commented Jul 31, 2018

Dealing with "Work In Progress" pull requests

Choose a reason for hiding this comment

lunny commented Aug 1, 2018

JulienTant commented Aug 1, 2018

Choose a reason for hiding this comment

adelowo Aug 2, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lunny commented Aug 13, 2018

lafriks left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JulienTant commented Aug 13, 2018

lafriks left a comment

Choose a reason for hiding this comment

JulienTant commented Jul 27, 2018 •

edited

Loading

codecov-io commented Jul 27, 2018 •

edited

Loading

adelowo Jul 30, 2018 •

edited

Loading

JulienTant commented Jul 30, 2018 •

edited

Loading

adelowo Aug 2, 2018 •

edited

Loading