Remove requireChangeId.

The option was deprecated in Ie5cd0c2b5 ("Mark requireChangeId as
deprecated").

Change-Id: I2c407bdded0f83e8264310d9ff53c3c6e85e8f4a
This commit is contained in:
Han-Wen Nienhuys
2019-07-15 15:13:27 +02:00
parent d32d57639e
commit ec79eb9682
26 changed files with 15 additions and 244 deletions

View File

@@ -157,32 +157,6 @@ is the author of the commit. Its main purpose is to improve tracking of who
did what, especially with patches. Default is `INHERIT`, which means that this did what, especially with patches. Default is `INHERIT`, which means that this
property is inherited from the parent project. property is inherited from the parent project.
[[receive.requireChangeId]]receive.requireChangeId::
+
The `Require Change-Id in commit message` option defines whether a
link:user-changeid.html[Change-Id] in the commit message is required
for pushing a commit for review. If this option is set, trying to push
a commit for review that doesn't contain a Change-Id in the commit
message fails with link:error-missing-changeid.html[missing Change-Id
in commit message footer].
It is recommended to set this option and use a
link:user-changeid.html#create[commit-msg hook] (or other client side
tooling like EGit) to automatically generate Change-Id's for new
commits. This way the Change-Id is automatically in place when changes
are reworked or rebased and uploading new patch sets gets easy.
If this option is not set, commits can be uploaded without a Change-Id,
but then users have to remember to copy the assigned Change-Id from the
change screen and insert it manually into the commit message when they
want to upload a second patch set.
Default is `INHERIT`, which means that this property is inherited from
the parent project. The global default for new hosts is `true`
This option is deprecated and future releases will behave as if this
is always `true`.
[[receive.maxObjectSizeLimit]]receive.maxObjectSizeLimit:: [[receive.maxObjectSizeLimit]]receive.maxObjectSizeLimit::
+ +
Maximum allowed Git object size that receive-pack will accept. If an object Maximum allowed Git object size that receive-pack will accept. If an object

View File

@@ -824,11 +824,6 @@ read access to `refs/meta/config`.
"configured_value": "INHERIT", "configured_value": "INHERIT",
"inherited_value": false "inherited_value": false
}, },
"require_change_id": {
"value": false,
"configured_value": "FALSE",
"inherited_value": true
},
"max_object_size_limit": { "max_object_size_limit": {
"value": "15m", "value": "15m",
"configured_value": "15m", "configured_value": "15m",
@@ -887,7 +882,6 @@ link:#config-input[ConfigInput] entity.
"enable_signed_push": "INHERIT", "enable_signed_push": "INHERIT",
"require_signed_push": "INHERIT", "require_signed_push": "INHERIT",
"reject_implicit_merges": "INHERIT", "reject_implicit_merges": "INHERIT",
"require_change_id": "TRUE",
"max_object_size_limit": "10m", "max_object_size_limit": "10m",
"submit_type": "REBASE_IF_NECESSARY", "submit_type": "REBASE_IF_NECESSARY",
"state": "ACTIVE" "state": "ACTIVE"
@@ -925,11 +919,6 @@ ConfigInfo] entity.
"configured_value": "INHERIT", "configured_value": "INHERIT",
"inherited_value": false "inherited_value": false
}, },
"require_change_id": {
"value": true,
"configured_value": "TRUE",
"inherited_value": true
},
"enable_signed_push": { "enable_signed_push": {
"value": true, "value": true,
"configured_value": "INHERIT", "configured_value": "INHERIT",
@@ -3094,12 +3083,6 @@ the uploader in the commit message.
|`create_new_change_for_all_not_in_target` |optional| |`create_new_change_for_all_not_in_target` |optional|
link:#inherited-boolean-info[InheritedBooleanInfo] that tells whether link:#inherited-boolean-info[InheritedBooleanInfo] that tells whether
a new change is created for every commit not in target branch. a new change is created for every commit not in target branch.
|`require_change_id` |optional|
link:#inherited-boolean-info[InheritedBooleanInfo] that tells whether a
valid link:user-changeid.html[Change-Id] footer in any commit uploaded
for review is required. This does not apply to commits pushed directly
to a branch or tag. This property is deprecated and will be removed in
a future release.
|`enable_signed_push`|optional, not set if signed push is disabled| |`enable_signed_push`|optional, not set if signed push is disabled|
link:#inherited-boolean-info[InheritedBooleanInfo] that tells whether link:#inherited-boolean-info[InheritedBooleanInfo] that tells whether
signed push validation is enabled on the project. signed push validation is enabled on the project.
@@ -3180,14 +3163,6 @@ Whether a new change will be created for every commit not in target
branch. + branch. +
Can be `TRUE`, `FALSE` or `INHERIT`. + Can be `TRUE`, `FALSE` or `INHERIT`. +
If not set, this setting is not updated. If not set, this setting is not updated.
|`require_change_id` |optional|
Whether a valid link:user-changeid.html[Change-Id] footer in any commit
uploaded for review is required. This does not apply to commits pushed
directly to a branch or tag. +
Can be `TRUE`, `FALSE` or `INHERIT`. +
If not set, this setting is not updated.
This property is deprecated and will be removed in
a future release.
|`reject_implicit_merges` |optional| |`reject_implicit_merges` |optional|
Whether a check for implicit merges will be performed when changes Whether a check for implicit merges will be performed when changes
are pushed for review. + are pushed for review. +
@@ -3553,11 +3528,6 @@ for the project (`TRUE`, `FALSE`, `INHERIT`).
Whether content merge should be enabled for the project (`TRUE`, Whether content merge should be enabled for the project (`TRUE`,
`FALSE`, `INHERIT`). + `FALSE`, `INHERIT`). +
`FALSE`, if the `submit_type` is `FAST_FORWARD_ONLY`. `FALSE`, if the `submit_type` is `FAST_FORWARD_ONLY`.
|`require_change_id` |`INHERIT` if not set|
Whether the usage of Change-Ids is required for the project (`TRUE`,
`FALSE`, `INHERIT`).
This property is deprecated and will be removed in
a future release.
|`enable_signed_push` |`INHERIT` if not set| |`enable_signed_push` |`INHERIT` if not set|
Whether signed push validation is enabled on the project (`TRUE`, Whether signed push validation is enabled on the project (`TRUE`,
`FALSE`, `INHERIT`). `FALSE`, `INHERIT`).

View File

@@ -888,15 +888,6 @@ public abstract class AbstractDaemonTest {
} }
} }
protected void setRequireChangeId(InheritableBoolean value) throws Exception {
try (MetaDataUpdate md = metaDataUpdateFactory.create(project)) {
ProjectConfig config = projectConfigFactory.read(md);
config.getProject().setBooleanConfig(BooleanProjectConfig.REQUIRE_CHANGE_ID, value);
config.commit(md);
projectCache.evict(config.getProject());
}
}
protected PushOneCommit.Result pushTo(String ref) throws Exception { protected PushOneCommit.Result pushTo(String ref) throws Exception {
PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo); PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo);
return push.to(ref); return push.to(ref);

View File

@@ -43,8 +43,6 @@ public @interface TestProjectInput {
InheritableBoolean useContentMerge() default InheritableBoolean.INHERIT; InheritableBoolean useContentMerge() default InheritableBoolean.INHERIT;
InheritableBoolean requireChangeId() default InheritableBoolean.INHERIT;
InheritableBoolean rejectEmptyCommit() default InheritableBoolean.INHERIT; InheritableBoolean rejectEmptyCommit() default InheritableBoolean.INHERIT;
InheritableBoolean enableSignedPush() default InheritableBoolean.INHERIT; InheritableBoolean enableSignedPush() default InheritableBoolean.INHERIT;

View File

@@ -29,7 +29,6 @@ public class ConfigInfo {
public InheritedBooleanInfo useContentMerge; public InheritedBooleanInfo useContentMerge;
public InheritedBooleanInfo useSignedOffBy; public InheritedBooleanInfo useSignedOffBy;
public InheritedBooleanInfo createNewChangeForAllNotInTarget; public InheritedBooleanInfo createNewChangeForAllNotInTarget;
public InheritedBooleanInfo requireChangeId;
public InheritedBooleanInfo enableSignedPush; public InheritedBooleanInfo enableSignedPush;
public InheritedBooleanInfo requireSignedPush; public InheritedBooleanInfo requireSignedPush;
public InheritedBooleanInfo rejectImplicitMerges; public InheritedBooleanInfo rejectImplicitMerges;

View File

@@ -25,7 +25,6 @@ public class ConfigInput {
public InheritableBoolean useContentMerge; public InheritableBoolean useContentMerge;
public InheritableBoolean useSignedOffBy; public InheritableBoolean useSignedOffBy;
public InheritableBoolean createNewChangeForAllNotInTarget; public InheritableBoolean createNewChangeForAllNotInTarget;
public InheritableBoolean requireChangeId;
public InheritableBoolean enableSignedPush; public InheritableBoolean enableSignedPush;
public InheritableBoolean requireSignedPush; public InheritableBoolean requireSignedPush;
public InheritableBoolean rejectImplicitMerges; public InheritableBoolean rejectImplicitMerges;

View File

@@ -31,7 +31,6 @@ public class ProjectInput {
public InheritableBoolean useContributorAgreements; public InheritableBoolean useContributorAgreements;
public InheritableBoolean useSignedOffBy; public InheritableBoolean useSignedOffBy;
public InheritableBoolean useContentMerge; public InheritableBoolean useContentMerge;
public InheritableBoolean requireChangeId;
public InheritableBoolean createNewChangeForAllNotInTarget; public InheritableBoolean createNewChangeForAllNotInTarget;
public InheritableBoolean rejectEmptyCommit; public InheritableBoolean rejectEmptyCommit;
public InheritableBoolean enableSignedPush; public InheritableBoolean enableSignedPush;

View File

@@ -32,7 +32,6 @@ public enum BooleanProjectConfig {
USE_CONTRIBUTOR_AGREEMENTS("receive", "requireContributorAgreement"), USE_CONTRIBUTOR_AGREEMENTS("receive", "requireContributorAgreement"),
USE_SIGNED_OFF_BY("receive", "requireSignedOffBy"), USE_SIGNED_OFF_BY("receive", "requireSignedOffBy"),
USE_CONTENT_MERGE("submit", "mergeContent"), USE_CONTENT_MERGE("submit", "mergeContent"),
REQUIRE_CHANGE_ID("receive", "requireChangeId"),
CREATE_NEW_CHANGE_FOR_ALL_NOT_IN_TARGET("receive", "createNewChangeForAllNotInTarget"), CREATE_NEW_CHANGE_FOR_ALL_NOT_IN_TARGET("receive", "createNewChangeForAllNotInTarget"),
ENABLE_SIGNED_PUSH("receive", "enableSignedPush"), ENABLE_SIGNED_PUSH("receive", "enableSignedPush"),
REQUIRE_SIGNED_PUSH("receive", "requireSignedPush"), REQUIRE_SIGNED_PUSH("receive", "requireSignedPush"),

View File

@@ -307,10 +307,8 @@ public class CommitValidators {
Type.ERROR)); Type.ERROR));
throw new CommitValidationException(CHANGE_ID_ABOVE_FOOTER_MSG, messages); throw new CommitValidationException(CHANGE_ID_ABOVE_FOOTER_MSG, messages);
} }
if (projectState.is(BooleanProjectConfig.REQUIRE_CHANGE_ID)) {
messages.add(getMissingChangeIdErrorMsg(MISSING_CHANGE_ID_MSG)); messages.add(getMissingChangeIdErrorMsg(MISSING_CHANGE_ID_MSG));
throw new CommitValidationException(MISSING_CHANGE_ID_MSG, messages); throw new CommitValidationException(MISSING_CHANGE_ID_MSG, messages);
}
} else if (idList.size() > 1) { } else if (idList.size() > 1) {
throw new CommitValidationException(MULTIPLE_CHANGE_ID_MSG, messages); throw new CommitValidationException(MULTIPLE_CHANGE_ID_MSG, messages);
} else { } else {

View File

@@ -38,9 +38,6 @@ public class BooleanProjectConfigTransformations {
.put( .put(
BooleanProjectConfig.USE_CONTENT_MERGE, BooleanProjectConfig.USE_CONTENT_MERGE,
new Mapper(i -> i.useContentMerge, (i, v) -> i.useContentMerge = v)) new Mapper(i -> i.useContentMerge, (i, v) -> i.useContentMerge = v))
.put(
BooleanProjectConfig.REQUIRE_CHANGE_ID,
new Mapper(i -> i.requireChangeId, (i, v) -> i.requireChangeId = v))
.put( .put(
BooleanProjectConfig.CREATE_NEW_CHANGE_FOR_ALL_NOT_IN_TARGET, BooleanProjectConfig.CREATE_NEW_CHANGE_FOR_ALL_NOT_IN_TARGET,
new Mapper( new Mapper(

View File

@@ -33,7 +33,6 @@ public class CreateProjectArgs {
public List<String> branch; public List<String> branch;
public InheritableBoolean contentMerge; public InheritableBoolean contentMerge;
public InheritableBoolean newChangeForAllNotInTarget; public InheritableBoolean newChangeForAllNotInTarget;
public InheritableBoolean changeIdRequired;
public InheritableBoolean rejectEmptyCommit; public InheritableBoolean rejectEmptyCommit;
public InheritableBoolean enableSignedPush; public InheritableBoolean enableSignedPush;
public InheritableBoolean requireSignedPush; public InheritableBoolean requireSignedPush;
@@ -44,7 +43,6 @@ public class CreateProjectArgs {
contributorAgreements = InheritableBoolean.INHERIT; contributorAgreements = InheritableBoolean.INHERIT;
signedOffBy = InheritableBoolean.INHERIT; signedOffBy = InheritableBoolean.INHERIT;
contentMerge = InheritableBoolean.INHERIT; contentMerge = InheritableBoolean.INHERIT;
changeIdRequired = InheritableBoolean.INHERIT;
newChangeForAllNotInTarget = InheritableBoolean.INHERIT; newChangeForAllNotInTarget = InheritableBoolean.INHERIT;
enableSignedPush = InheritableBoolean.INHERIT; enableSignedPush = InheritableBoolean.INHERIT;
requireSignedPush = InheritableBoolean.INHERIT; requireSignedPush = InheritableBoolean.INHERIT;

View File

@@ -152,7 +152,6 @@ public class ProjectCreator {
newProject.setBooleanConfig( newProject.setBooleanConfig(
BooleanProjectConfig.CREATE_NEW_CHANGE_FOR_ALL_NOT_IN_TARGET, BooleanProjectConfig.CREATE_NEW_CHANGE_FOR_ALL_NOT_IN_TARGET,
args.newChangeForAllNotInTarget); args.newChangeForAllNotInTarget);
newProject.setBooleanConfig(BooleanProjectConfig.REQUIRE_CHANGE_ID, args.changeIdRequired);
newProject.setBooleanConfig(BooleanProjectConfig.REJECT_EMPTY_COMMIT, args.rejectEmptyCommit); newProject.setBooleanConfig(BooleanProjectConfig.REJECT_EMPTY_COMMIT, args.rejectEmptyCommit);
newProject.setMaxObjectSizeLimit(args.maxObjectSizeLimit); newProject.setMaxObjectSizeLimit(args.maxObjectSizeLimit);
newProject.setBooleanConfig(BooleanProjectConfig.ENABLE_SIGNED_PUSH, args.enableSignedPush); newProject.setBooleanConfig(BooleanProjectConfig.ENABLE_SIGNED_PUSH, args.enableSignedPush);

View File

@@ -22,7 +22,6 @@ import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.BooleanProjectConfig;
import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.CurrentUser;
@@ -111,10 +110,7 @@ public class PutMessage
String sanitizedCommitMessage = CommitMessageUtil.checkAndSanitizeCommitMessage(input.message); String sanitizedCommitMessage = CommitMessageUtil.checkAndSanitizeCommitMessage(input.message);
ensureCanEditCommitMessage(resource.getNotes()); ensureCanEditCommitMessage(resource.getNotes());
ensureChangeIdIsCorrect( ensureChangeIdIsCorrect(resource.getChange().getKey().get(), sanitizedCommitMessage);
projectCache.checkedGet(resource.getProject()).is(BooleanProjectConfig.REQUIRE_CHANGE_ID),
resource.getChange().getKey().get(),
sanitizedCommitMessage);
try (Repository repository = repositoryManager.openRepository(resource.getProject()); try (Repository repository = repositoryManager.openRepository(resource.getProject());
RevWalk revWalk = new RevWalk(repository); RevWalk revWalk = new RevWalk(repository);
@@ -193,8 +189,7 @@ public class PutMessage
} }
} }
private static void ensureChangeIdIsCorrect( private static void ensureChangeIdIsCorrect(String currentChangeId, String newCommitMessage)
boolean requireChangeId, String currentChangeId, String newCommitMessage)
throws ResourceConflictException, BadRequestException { throws ResourceConflictException, BadRequestException {
RevCommit revCommit = RevCommit revCommit =
RevCommit.parse( RevCommit.parse(
@@ -204,7 +199,7 @@ public class PutMessage
CommitMessageUtil.checkAndSanitizeCommitMessage(revCommit.getShortMessage()); CommitMessageUtil.checkAndSanitizeCommitMessage(revCommit.getShortMessage());
List<String> changeIdFooters = revCommit.getFooterLines(FooterConstants.CHANGE_ID); List<String> changeIdFooters = revCommit.getFooterLines(FooterConstants.CHANGE_ID);
if (requireChangeId && changeIdFooters.isEmpty()) { if (changeIdFooters.isEmpty()) {
throw new ResourceConflictException("missing Change-Id footer"); throw new ResourceConflictException("missing Change-Id footer");
} }
if (!changeIdFooters.isEmpty() && !changeIdFooters.get(0).equals(currentChangeId)) { if (!changeIdFooters.isEmpty() && !changeIdFooters.get(0).equals(currentChangeId)) {

View File

@@ -147,8 +147,6 @@ public class CreateProject
args.newChangeForAllNotInTarget = args.newChangeForAllNotInTarget =
MoreObjects.firstNonNull( MoreObjects.firstNonNull(
input.createNewChangeForAllNotInTarget, InheritableBoolean.INHERIT); input.createNewChangeForAllNotInTarget, InheritableBoolean.INHERIT);
args.changeIdRequired =
MoreObjects.firstNonNull(input.requireChangeId, InheritableBoolean.INHERIT);
args.rejectEmptyCommit = args.rejectEmptyCommit =
MoreObjects.firstNonNull(input.rejectEmptyCommit, InheritableBoolean.INHERIT); MoreObjects.firstNonNull(input.rejectEmptyCommit, InheritableBoolean.INHERIT);
args.enableSignedPush = args.enableSignedPush =

View File

@@ -33,8 +33,6 @@ public abstract class AllProjectsInput {
public static final ImmutableMap<BooleanProjectConfig, InheritableBoolean> public static final ImmutableMap<BooleanProjectConfig, InheritableBoolean>
DEFAULT_BOOLEAN_PROJECT_CONFIGS = DEFAULT_BOOLEAN_PROJECT_CONFIGS =
ImmutableMap.of( ImmutableMap.of(
BooleanProjectConfig.REQUIRE_CHANGE_ID,
InheritableBoolean.TRUE,
BooleanProjectConfig.USE_CONTENT_MERGE, BooleanProjectConfig.USE_CONTENT_MERGE,
InheritableBoolean.TRUE, InheritableBoolean.TRUE,
BooleanProjectConfig.USE_CONTRIBUTOR_AGREEMENTS, BooleanProjectConfig.USE_CONTRIBUTOR_AGREEMENTS,

View File

@@ -40,7 +40,6 @@ public class AllProjectsCreatorTestUtil {
"[receive]", "[receive]",
" requireContributorAgreement = false", " requireContributorAgreement = false",
" requireSignedOffBy = false", " requireSignedOffBy = false",
" requireChangeId = true",
" enableSignedPush = false"); " enableSignedPush = false");
private static final ImmutableList<String> DEFAULT_ALL_PROJECTS_SUBMIT_SECTION = private static final ImmutableList<String> DEFAULT_ALL_PROJECTS_SUBMIT_SECTION =
ImmutableList.of("[submit]", " mergeContent = true"); ImmutableList.of("[submit]", " mergeContent = true");

View File

@@ -91,9 +91,6 @@ final class CreateProjectCommand extends SshCommand {
@Option(name = "--content-merge", usage = "allow automatic conflict resolving within files") @Option(name = "--content-merge", usage = "allow automatic conflict resolving within files")
private InheritableBoolean contentMerge = InheritableBoolean.INHERIT; private InheritableBoolean contentMerge = InheritableBoolean.INHERIT;
@Option(name = "--change-id", usage = "if change-id is required")
private InheritableBoolean requireChangeID = InheritableBoolean.INHERIT;
@Option(name = "--reject-empty-commit", usage = "if empty commits should be rejected on submit") @Option(name = "--reject-empty-commit", usage = "if empty commits should be rejected on submit")
private InheritableBoolean rejectEmptyCommit = InheritableBoolean.INHERIT; private InheritableBoolean rejectEmptyCommit = InheritableBoolean.INHERIT;
@@ -123,14 +120,6 @@ final class CreateProjectCommand extends SshCommand {
contentMerge = InheritableBoolean.TRUE; contentMerge = InheritableBoolean.TRUE;
} }
@Option(
name = "--require-change-id",
aliases = {"--id"},
usage = "if change-id is required")
void setRequireChangeId(@SuppressWarnings("unused") boolean on) {
requireChangeID = InheritableBoolean.TRUE;
}
@Option( @Option(
name = "--create-new-change-for-all-not-in-target", name = "--create-new-change-for-all-not-in-target",
aliases = {"--ncfa"}, aliases = {"--ncfa"},
@@ -186,7 +175,6 @@ final class CreateProjectCommand extends SshCommand {
input.useContributorAgreements = contributorAgreements; input.useContributorAgreements = contributorAgreements;
input.useSignedOffBy = signedOffBy; input.useSignedOffBy = signedOffBy;
input.useContentMerge = contentMerge; input.useContentMerge = contentMerge;
input.requireChangeId = requireChangeID;
input.createNewChangeForAllNotInTarget = createNewChangeForAllNotInTarget; input.createNewChangeForAllNotInTarget = createNewChangeForAllNotInTarget;
input.branches = branch; input.branches = branch;
input.createEmptyCommit = createEmptyCommit; input.createEmptyCommit = createEmptyCommit;

View File

@@ -56,9 +56,6 @@ final class SetProjectCommand extends SshCommand {
@Option(name = "--content-merge", usage = "allow automatic conflict resolving within files") @Option(name = "--content-merge", usage = "allow automatic conflict resolving within files")
private InheritableBoolean contentMerge; private InheritableBoolean contentMerge;
@Option(name = "--change-id", usage = "if change-id is required")
private InheritableBoolean requireChangeID;
@Option( @Option(
name = "--use-contributor-agreements", name = "--use-contributor-agreements",
aliases = {"--ca"}, aliases = {"--ca"},
@@ -103,22 +100,6 @@ final class SetProjectCommand extends SshCommand {
contentMerge = InheritableBoolean.FALSE; contentMerge = InheritableBoolean.FALSE;
} }
@Option(
name = "--require-change-id",
aliases = {"--id"},
usage = "if change-id is required")
void setRequireChangeId(@SuppressWarnings("unused") boolean on) {
requireChangeID = InheritableBoolean.TRUE;
}
@Option(
name = "--no-change-id",
aliases = {"--nid"},
usage = "if change-id is not required")
void setNoChangeId(@SuppressWarnings("unused") boolean on) {
requireChangeID = InheritableBoolean.FALSE;
}
@Option( @Option(
name = "--project-state", name = "--project-state",
aliases = {"--ps"}, aliases = {"--ps"},
@@ -133,7 +114,6 @@ final class SetProjectCommand extends SshCommand {
@Override @Override
protected void run() throws Failure { protected void run() throws Failure {
ConfigInput configInput = new ConfigInput(); ConfigInput configInput = new ConfigInput();
configInput.requireChangeId = requireChangeID;
configInput.submitType = submitType; configInput.submitType = submitType;
configInput.useContentMerge = contentMerge; configInput.useContentMerge = contentMerge;
configInput.useContributorAgreements = contributorAgreements; configInput.useContributorAgreements = contributorAgreements;

View File

@@ -3776,24 +3776,6 @@ public class ChangeIT extends AbstractDaemonTest {
.isEqualTo(ChangeMessagesUtil.TAG_UPLOADED_WIP_PATCH_SET); .isEqualTo(ChangeMessagesUtil.TAG_UPLOADED_WIP_PATCH_SET);
} }
@Test
public void changeCommitMessageWithNoChangeIdSucceedsIfChangeIdNotRequired() throws Exception {
ConfigInput configInput = new ConfigInput();
configInput.requireChangeId = InheritableBoolean.FALSE;
gApi.projects().name(project.get()).config(configInput);
PushOneCommit.Result r = createChange();
r.assertOkStatus();
assertThat(getCommitMessage(r.getChangeId()))
.isEqualTo("test commit\n\nChange-Id: " + r.getChangeId() + "\n");
String newMessage = "modified commit\n";
gApi.changes().id(r.getChangeId()).setMessage(newMessage);
RevisionApi rApi = gApi.changes().id(r.getChangeId()).current();
assertThat(rApi.files().keySet()).containsExactly("/COMMIT_MSG", "a.txt");
assertThat(getCommitMessage(r.getChangeId())).isEqualTo(newMessage);
}
@Test @Test
public void changeCommitMessageWithNoChangeIdFails() throws Exception { public void changeCommitMessageWithNoChangeIdFails() throws Exception {
PushOneCommit.Result r = createChange(); PushOneCommit.Result r = createChange();

View File

@@ -27,7 +27,6 @@ import com.google.gerrit.extensions.api.projects.CheckProjectInput;
import com.google.gerrit.extensions.api.projects.CheckProjectInput.AutoCloseableChangesCheckInput; import com.google.gerrit.extensions.api.projects.CheckProjectInput.AutoCloseableChangesCheckInput;
import com.google.gerrit.extensions.api.projects.CheckProjectResultInfo; import com.google.gerrit.extensions.api.projects.CheckProjectResultInfo;
import com.google.gerrit.extensions.client.ChangeStatus; import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.client.InheritableBoolean;
import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException; import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
@@ -66,7 +65,7 @@ public class CheckProjectIT extends AbstractDaemonTest {
@Test @Test
public void detectAutoCloseableChangeByCommit() throws Exception { public void detectAutoCloseableChangeByCommit() throws Exception {
RevCommit commit = pushCommitWithoutChangeIdForReview(); RevCommit commit = pushCommitForReview();
ChangeInfo change = ChangeInfo change =
Iterables.getOnlyElement(gApi.changes().query("commit:" + commit.name()).get()); Iterables.getOnlyElement(gApi.changes().query("commit:" + commit.name()).get());
@@ -90,7 +89,7 @@ public class CheckProjectIT extends AbstractDaemonTest {
@Test @Test
public void fixAutoCloseableChangeByCommit() throws Exception { public void fixAutoCloseableChangeByCommit() throws Exception {
RevCommit commit = pushCommitWithoutChangeIdForReview(); RevCommit commit = pushCommitForReview();
ChangeInfo change = ChangeInfo change =
Iterables.getOnlyElement(gApi.changes().query("commit:" + commit.name()).get()); Iterables.getOnlyElement(gApi.changes().query("commit:" + commit.name()).get());
@@ -280,8 +279,7 @@ public class CheckProjectIT extends AbstractDaemonTest {
+ ProjectsConsistencyChecker.AUTO_CLOSE_MAX_COMMITS_LIMIT); + ProjectsConsistencyChecker.AUTO_CLOSE_MAX_COMMITS_LIMIT);
} }
private RevCommit pushCommitWithoutChangeIdForReview() throws Exception { private RevCommit pushCommitForReview() throws Exception {
setRequireChangeId(InheritableBoolean.FALSE);
RevCommit commit = RevCommit commit =
testRepo testRepo
.branch("HEAD") .branch("HEAD")

View File

@@ -316,7 +316,6 @@ public class ProjectIT extends AbstractDaemonTest {
assertThat(info.useSignedOffBy.configuredValue).isEqualTo(input.useSignedOffBy); assertThat(info.useSignedOffBy.configuredValue).isEqualTo(input.useSignedOffBy);
assertThat(info.createNewChangeForAllNotInTarget.configuredValue) assertThat(info.createNewChangeForAllNotInTarget.configuredValue)
.isEqualTo(input.createNewChangeForAllNotInTarget); .isEqualTo(input.createNewChangeForAllNotInTarget);
assertThat(info.requireChangeId.configuredValue).isEqualTo(input.requireChangeId);
assertThat(info.rejectImplicitMerges.configuredValue).isEqualTo(input.rejectImplicitMerges); assertThat(info.rejectImplicitMerges.configuredValue).isEqualTo(input.rejectImplicitMerges);
assertThat(info.enableReviewerByEmail.configuredValue).isEqualTo(input.enableReviewerByEmail); assertThat(info.enableReviewerByEmail.configuredValue).isEqualTo(input.enableReviewerByEmail);
assertThat(info.createNewChangeForAllNotInTarget.configuredValue) assertThat(info.createNewChangeForAllNotInTarget.configuredValue)
@@ -346,7 +345,6 @@ public class ProjectIT extends AbstractDaemonTest {
assertThat(info.useSignedOffBy.configuredValue).isEqualTo(input.useSignedOffBy); assertThat(info.useSignedOffBy.configuredValue).isEqualTo(input.useSignedOffBy);
assertThat(info.createNewChangeForAllNotInTarget.configuredValue) assertThat(info.createNewChangeForAllNotInTarget.configuredValue)
.isEqualTo(input.createNewChangeForAllNotInTarget); .isEqualTo(input.createNewChangeForAllNotInTarget);
assertThat(info.requireChangeId.configuredValue).isEqualTo(input.requireChangeId);
assertThat(info.rejectImplicitMerges.configuredValue).isEqualTo(input.rejectImplicitMerges); assertThat(info.rejectImplicitMerges.configuredValue).isEqualTo(input.rejectImplicitMerges);
assertThat(info.enableReviewerByEmail.configuredValue).isEqualTo(input.enableReviewerByEmail); assertThat(info.enableReviewerByEmail.configuredValue).isEqualTo(input.enableReviewerByEmail);
assertThat(info.createNewChangeForAllNotInTarget.configuredValue) assertThat(info.createNewChangeForAllNotInTarget.configuredValue)
@@ -683,7 +681,6 @@ public class ProjectIT extends AbstractDaemonTest {
input.useContentMerge = InheritableBoolean.TRUE; input.useContentMerge = InheritableBoolean.TRUE;
input.useSignedOffBy = InheritableBoolean.TRUE; input.useSignedOffBy = InheritableBoolean.TRUE;
input.createNewChangeForAllNotInTarget = InheritableBoolean.TRUE; input.createNewChangeForAllNotInTarget = InheritableBoolean.TRUE;
input.requireChangeId = InheritableBoolean.TRUE;
input.rejectImplicitMerges = InheritableBoolean.TRUE; input.rejectImplicitMerges = InheritableBoolean.TRUE;
input.enableReviewerByEmail = InheritableBoolean.TRUE; input.enableReviewerByEmail = InheritableBoolean.TRUE;
input.createNewChangeForAllNotInTarget = InheritableBoolean.TRUE; input.createNewChangeForAllNotInTarget = InheritableBoolean.TRUE;

View File

@@ -89,7 +89,6 @@ import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.git.ObjectIds; import com.google.gerrit.git.ObjectIds;
import com.google.gerrit.mail.Address; import com.google.gerrit.mail.Address;
import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.BooleanProjectConfig;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage; import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
@@ -124,7 +123,6 @@ import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.AnyObjectId; import org.eclipse.jgit.lib.AnyObjectId;
import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.RefUpdate; import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.RefUpdate.Result; import org.eclipse.jgit.lib.RefUpdate.Result;
import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.Repository;
@@ -458,20 +456,6 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
.isEqualTo("Change has been successfully pushed."); .isEqualTo("Change has been successfully pushed.");
} }
@Test
public void pushWithoutChangeIdDeprecated() throws Exception {
setRequireChangeId(InheritableBoolean.FALSE);
testRepo
.branch("HEAD")
.commit()
.message("A change")
.author(admin.newIdent())
.committer(new PersonIdent(admin.newIdent(), testRepo.getDate()))
.create();
PushResult result = pushHead(testRepo, "refs/for/master");
assertThat(result.getMessages()).contains("warning: pushing without Change-Id is deprecated");
}
@Test @Test
public void autocloseByChangeId() throws Exception { public void autocloseByChangeId() throws Exception {
// Create a change // Create a change
@@ -1572,9 +1556,6 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
RevCommit c = createCommit(testRepo, "Message without Change-Id"); RevCommit c = createCommit(testRepo, "Message without Change-Id");
assertThat(GitUtil.getChangeId(testRepo, c)).isEmpty(); assertThat(GitUtil.getChangeId(testRepo, c)).isEmpty();
pushForReviewRejected(testRepo, "missing Change-Id in message footer"); pushForReviewRejected(testRepo, "missing Change-Id in message footer");
setRequireChangeId(InheritableBoolean.FALSE);
pushForReviewOk(testRepo);
} }
@Test @Test
@@ -1598,9 +1579,6 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
+ "More text, uh oh.\n"); + "More text, uh oh.\n");
assertThat(GitUtil.getChangeId(testRepo, c)).isEmpty(); assertThat(GitUtil.getChangeId(testRepo, c)).isEmpty();
pushForReviewRejected(testRepo, "Change-Id must be in message footer"); pushForReviewRejected(testRepo, "Change-Id must be in message footer");
setRequireChangeId(InheritableBoolean.FALSE);
pushForReviewRejected(testRepo, "Change-Id must be in message footer");
} }
@Test @Test
@@ -1637,9 +1615,6 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
+ "Change-Id: I10f98c2ef76e52e23aa23be5afeb71e40b350e86\n" + "Change-Id: I10f98c2ef76e52e23aa23be5afeb71e40b350e86\n"
+ "Change-Id: Ie9a132e107def33bdd513b7854b50de911edba0a\n"); + "Change-Id: Ie9a132e107def33bdd513b7854b50de911edba0a\n");
pushForReviewRejected(testRepo, "multiple Change-Id lines in message footer"); pushForReviewRejected(testRepo, "multiple Change-Id lines in message footer");
setRequireChangeId(InheritableBoolean.FALSE);
pushForReviewRejected(testRepo, "multiple Change-Id lines in message footer");
} }
@Test @Test
@@ -1656,9 +1631,6 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
private void testpushWithInvalidChangeId() throws Exception { private void testpushWithInvalidChangeId() throws Exception {
createCommit(testRepo, "Message with invalid Change-Id\n\nChange-Id: X\n"); createCommit(testRepo, "Message with invalid Change-Id\n\nChange-Id: X\n");
pushForReviewRejected(testRepo, "invalid Change-Id line format in message footer"); pushForReviewRejected(testRepo, "invalid Change-Id line format in message footer");
setRequireChangeId(InheritableBoolean.FALSE);
pushForReviewRejected(testRepo, "invalid Change-Id line format in message footer");
} }
@Test @Test
@@ -1680,18 +1652,12 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
+ "\n" + "\n"
+ "Change-Id: I0000000000000000000000000000000000000000\n"); + "Change-Id: I0000000000000000000000000000000000000000\n");
pushForReviewRejected(testRepo, "invalid Change-Id line format in message footer"); pushForReviewRejected(testRepo, "invalid Change-Id line format in message footer");
setRequireChangeId(InheritableBoolean.FALSE);
pushForReviewRejected(testRepo, "invalid Change-Id line format in message footer");
} }
@Test @Test
public void pushWithChangeIdInSubjectLine() throws Exception { public void pushWithChangeIdInSubjectLine() throws Exception {
createCommit(testRepo, "Change-Id: I1234000000000000000000000000000000000000"); createCommit(testRepo, "Change-Id: I1234000000000000000000000000000000000000");
pushForReviewRejected(testRepo, "missing subject; Change-Id must be in message footer"); pushForReviewRejected(testRepo, "missing subject; Change-Id must be in message footer");
setRequireChangeId(InheritableBoolean.FALSE);
pushForReviewRejected(testRepo, "missing subject; Change-Id must be in message footer");
} }
@Test @Test
@@ -1704,19 +1670,6 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
createCommit(testRepo, commitChange1.getFullMessage()); createCommit(testRepo, commitChange1.getFullMessage());
pushForReviewRejected(
testRepo,
"same Change-Id in multiple changes.\n"
+ "Squash the commits with the same Change-Id or ensure Change-Ids are unique for each"
+ " commit");
try (ProjectConfigUpdate u = updateProject(project)) {
u.getConfig()
.getProject()
.setBooleanConfig(BooleanProjectConfig.REQUIRE_CHANGE_ID, InheritableBoolean.FALSE);
u.save();
}
pushForReviewRejected( pushForReviewRejected(
testRepo, testRepo,
"same Change-Id in multiple changes.\n" "same Change-Id in multiple changes.\n"
@@ -1730,19 +1683,6 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
createCommit(testRepo, commitChange1.getFullMessage()); createCommit(testRepo, commitChange1.getFullMessage());
pushForReviewRejected(
testRepo,
"same Change-Id in multiple changes.\n"
+ "Squash the commits with the same Change-Id or ensure Change-Ids are unique for each"
+ " commit");
try (ProjectConfigUpdate u = updateProject(project)) {
u.getConfig()
.getProject()
.setBooleanConfig(BooleanProjectConfig.REQUIRE_CHANGE_ID, InheritableBoolean.FALSE);
u.save();
}
pushForReviewRejected( pushForReviewRejected(
testRepo, testRepo,
"same Change-Id in multiple changes.\n" "same Change-Id in multiple changes.\n"

View File

@@ -218,7 +218,6 @@ public class CreateProjectIT extends AbstractDaemonTest {
in.useContributorAgreements = InheritableBoolean.TRUE; in.useContributorAgreements = InheritableBoolean.TRUE;
in.useSignedOffBy = InheritableBoolean.TRUE; in.useSignedOffBy = InheritableBoolean.TRUE;
in.useContentMerge = InheritableBoolean.TRUE; in.useContentMerge = InheritableBoolean.TRUE;
in.requireChangeId = InheritableBoolean.TRUE;
ProjectInfo p = gApi.projects().create(in).get(); ProjectInfo p = gApi.projects().create(in).get();
assertThat(p.name).isEqualTo(newProjectName); assertThat(p.name).isEqualTo(newProjectName);
Project project = projectCache.get(Project.nameKey(newProjectName)).getProject(); Project project = projectCache.get(Project.nameKey(newProjectName)).getProject();
@@ -231,8 +230,6 @@ public class CreateProjectIT extends AbstractDaemonTest {
.isEqualTo(in.useSignedOffBy); .isEqualTo(in.useSignedOffBy);
assertThat(project.getBooleanConfig(BooleanProjectConfig.USE_CONTENT_MERGE)) assertThat(project.getBooleanConfig(BooleanProjectConfig.USE_CONTENT_MERGE))
.isEqualTo(in.useContentMerge); .isEqualTo(in.useContentMerge);
assertThat(project.getBooleanConfig(BooleanProjectConfig.REQUIRE_CHANGE_ID))
.isEqualTo(in.requireChangeId);
} }
@Test @Test

View File

@@ -16,7 +16,7 @@ package com.google.gerrit.server.project;
import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage; import static com.google.common.truth.Truth.assertWithMessage;
import static com.google.gerrit.reviewdb.client.BooleanProjectConfig.REQUIRE_CHANGE_ID; import static com.google.gerrit.reviewdb.client.BooleanProjectConfig.USE_CONTRIBUTOR_AGREEMENTS;
import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables; import com.google.common.collect.Iterables;
@@ -612,13 +612,13 @@ public class ProjectConfigTest {
public void readAllProjectsBaseConfigFromSitePaths() throws Exception { public void readAllProjectsBaseConfigFromSitePaths() throws Exception {
ProjectConfig cfg = factory.create(ALL_PROJECTS); ProjectConfig cfg = factory.create(ALL_PROJECTS);
cfg.load(db); cfg.load(db);
assertThat(cfg.getProject().getBooleanConfig(REQUIRE_CHANGE_ID)) assertThat(cfg.getProject().getBooleanConfig(USE_CONTRIBUTOR_AGREEMENTS))
.isEqualTo(InheritableBoolean.INHERIT); .isEqualTo(InheritableBoolean.INHERIT);
writeDefaultAllProjectsConfig("[receive]", "requireChangeId = false"); writeDefaultAllProjectsConfig("[receive]", "requireContributorAgreement = false");
cfg.load(db); cfg.load(db);
assertThat(cfg.getProject().getBooleanConfig(REQUIRE_CHANGE_ID)) assertThat(cfg.getProject().getBooleanConfig(USE_CONTRIBUTOR_AGREEMENTS))
.isEqualTo(InheritableBoolean.FALSE); .isEqualTo(InheritableBoolean.FALSE);
} }
@@ -626,17 +626,17 @@ public class ProjectConfigTest {
public void readOtherProjectIgnoresAllProjectsBaseConfig() throws Exception { public void readOtherProjectIgnoresAllProjectsBaseConfig() throws Exception {
ProjectConfig cfg = factory.create(Project.nameKey("test")); ProjectConfig cfg = factory.create(Project.nameKey("test"));
cfg.load(db); cfg.load(db);
assertThat(cfg.getProject().getBooleanConfig(REQUIRE_CHANGE_ID)) assertThat(cfg.getProject().getBooleanConfig(USE_CONTRIBUTOR_AGREEMENTS))
.isEqualTo(InheritableBoolean.INHERIT); .isEqualTo(InheritableBoolean.INHERIT);
writeDefaultAllProjectsConfig("[receive]", "requireChangeId = false"); writeDefaultAllProjectsConfig("[receive]", "requireContributorAgreement = false");
cfg.load(db); cfg.load(db);
// If we went through ProjectState, then this would return FALSE, since project.config for // If we went through ProjectState, then this would return FALSE, since project.config for
// All-Projects would inherit from all_projects.config, and this project would inherit from // All-Projects would inherit from all_projects.config, and this project would inherit from
// All-Projects. But in ProjectConfig itself, there is no inheritance from All-Projects, so this // All-Projects. But in ProjectConfig itself, there is no inheritance from All-Projects, so this
// continues to return the default. // continues to return the default.
assertThat(cfg.getProject().getBooleanConfig(REQUIRE_CHANGE_ID)) assertThat(cfg.getProject().getBooleanConfig(USE_CONTRIBUTOR_AGREEMENTS))
.isEqualTo(InheritableBoolean.INHERIT); .isEqualTo(InheritableBoolean.INHERIT);
} }

View File

@@ -154,21 +154,6 @@ limitations under the License.
</gr-select> </gr-select>
</span> </span>
</section> </section>
<section>
<span class="title">Require Change-Id in commit message</span>
<span class="value">
<gr-select
id="requireChangeIdSelect"
bind-value="{{_repoConfig.require_change_id.configured_value}}">
<select disabled$="[[_readOnly]]">
<template is="dom-repeat"
items="[[_formatBooleanSelect(_repoConfig.require_change_id)]]">
<option value="[[item.value]]">[[item.label]]</option>
</template>
</select>
</gr-select>
</span>
</section>
<section <section
id="enableSignedPushSettings" id="enableSignedPushSettings"
class$="repositorySettings [[_computeRepositoriesClass(_repoConfig.enable_signed_push)]]"> class$="repositorySettings [[_computeRepositoriesClass(_repoConfig.enable_signed_push)]]">

View File

@@ -57,10 +57,6 @@ limitations under the License.
value: false, value: false,
configured_value: 'FALSE', configured_value: 'FALSE',
}, },
require_change_id: {
value: false,
configured_value: 'FALSE',
},
enable_signed_push: { enable_signed_push: {
value: false, value: false,
configured_value: 'FALSE', configured_value: 'FALSE',
@@ -322,7 +318,6 @@ limitations under the License.
use_content_merge: 'TRUE', use_content_merge: 'TRUE',
use_signed_off_by: 'TRUE', use_signed_off_by: 'TRUE',
create_new_change_for_all_not_in_target: 'TRUE', create_new_change_for_all_not_in_target: 'TRUE',
require_change_id: 'TRUE',
enable_signed_push: 'TRUE', enable_signed_push: 'TRUE',
require_signed_push: 'TRUE', require_signed_push: 'TRUE',
reject_implicit_merges: 'TRUE', reject_implicit_merges: 'TRUE',
@@ -352,8 +347,6 @@ limitations under the License.
configInputObj.use_content_merge; configInputObj.use_content_merge;
element.$.newChangeSelect.bindValue = element.$.newChangeSelect.bindValue =
configInputObj.create_new_change_for_all_not_in_target; configInputObj.create_new_change_for_all_not_in_target;
element.$.requireChangeIdSelect.bindValue =
configInputObj.require_change_id;
element.$.enableSignedPush.bindValue = element.$.enableSignedPush.bindValue =
configInputObj.enable_signed_push; configInputObj.enable_signed_push;
element.$.requireSignedPush.bindValue = element.$.requireSignedPush.bindValue =