Skip to content

introduces writeSignoffCommit and requireScore features #1205

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions src/main/distrib/data/defaults.properties
Original file line number Diff line number Diff line change
@@ -582,6 +582,13 @@ tickets.acceptNewPatchsets = true
# SINCE 1.4.0
tickets.requireApproval = false

# Set to a value >= 0 to require at least that cumulated score on a patchset to merge
# via the WebUI. Requires requireApproval set to true on a repository.
# Set to negative values to disable.
#
# SINCE 1.9.0
tickets.requireScore = -1

# Default setting to control how patchsets are merged to the integration branch.
# Valid values:
# MERGE_ALWAYS - Always merge with a merge commit. Every ticket will show up as a branch,
@@ -631,6 +638,23 @@ tickets.redis.url =
# SINCE 1.4.0
tickets.perPage = 25

# Appends the reviewers with a positive on a ticket's
# patchset in the form of 'value: author <author@example.com>'
# (where value is this # parameter's value) lines to the
# merge commit message when the WebUI merge button is clicked.
#
# Set to the desired value to enable, e.g.:
#
# tickets.writeSignoffCommit = Signed-off-by
#
# Defaults to being disabled, can be overridden on repositories.
#
# See the conventions on the meaning of possible values for this
# parameter: https://git.wiki.kernel.org/index.php/CommitMessageConventions
#
# SINCE 1.9.0
tickets.writeSignoffCommit =

# The folder where plugins are loaded from.
#
# SINCE 1.5.0
24 changes: 24 additions & 0 deletions src/main/java/com/gitblit/client/EditRepositoryDialog.java
Original file line number Diff line number Diff line change
@@ -96,6 +96,10 @@ public class EditRepositoryDialog extends JDialog {

private JCheckBox requireApproval;

private JComboBox requireScore;

private JComboBox writeSignoffCommit;

private JComboBox mergeToField;

private JCheckBox useIncrementalPushTags;
@@ -221,6 +225,19 @@ private void initialize(int protocolVersion, RepositoryModel anRepository) {
anRepository.acceptNewPatchsets);
requireApproval = new JCheckBox(Translation.get("gb.requireApprovalDescription"),
anRepository.requireApproval);
Integer [] scores = { -1, 0, 2, 4, 5, 6, 8 };
requireScore = new JComboBox(scores);
requireScore.setSelectedItem(anRepository.requireScore);
requireScore.setEnabled(anRepository.requireApproval);

String [] signoffCommitMsgs = {
null,
"Signed-off-by",
"Reviewed-by",
"Acked-by"
};
writeSignoffCommit = new JComboBox(signoffCommitMsgs);
writeSignoffCommit.setSelectedItem(anRepository.writeSignoffCommit);

if (ArrayUtils.isEmpty(anRepository.availableRefs)) {
mergeToField = new JComboBox();
@@ -330,6 +347,10 @@ public void itemStateChanged(ItemEvent e) {
acceptNewPatchsets));
fieldsPanel.add(newFieldPanel(Translation.get("gb.requireApproval"),
requireApproval));
fieldsPanel.add(newFieldPanel(Translation.get("gb.requireScore"),
requireScore));
fieldsPanel.add(newFieldPanel(Translation.get("gb.writeSignoffCommit"),
writeSignoffCommit));
fieldsPanel.add(newFieldPanel(Translation.get("gb.mergeTo"), mergeToField));
fieldsPanel
.add(newFieldPanel(Translation.get("gb.enableIncrementalPushTags"), useIncrementalPushTags));
@@ -588,6 +609,9 @@ private boolean validateFields() {
repository.acceptNewPatchsets = acceptNewPatchsets.isSelected();
repository.acceptNewTickets = acceptNewTickets.isSelected();
repository.requireApproval = requireApproval.isSelected();
repository.requireScore = (Integer) requireScore.getSelectedItem();
repository.writeSignoffCommit = writeSignoffCommit.getSelectedItem() == null ? null
: writeSignoffCommit.getSelectedItem().toString();
repository.mergeTo = mergeToField.getSelectedItem() == null ? null
: Repository.shortenRefName(mergeToField.getSelectedItem().toString());
repository.useIncrementalPushTags = useIncrementalPushTags.isSelected();
28 changes: 27 additions & 1 deletion src/main/java/com/gitblit/git/PatchsetReceivePack.java
Original file line number Diff line number Diff line change
@@ -1268,7 +1268,33 @@ private void updateReflog(RefUpdate ru) {
public MergeStatus merge(TicketModel ticket) {
PersonIdent committer = new PersonIdent(user.getDisplayName(), StringUtils.isEmpty(user.emailAddress) ? (user.username + "@gitblit") : user.emailAddress);
Patchset patchset = ticket.getCurrentPatchset();
String message = MessageFormat.format("Merged #{0,number,0} \"{1}\"", ticket.number, ticket.title);
StringBuilder messageBuilder = new StringBuilder();
messageBuilder.append(MessageFormat.format("Merged #{0,number,0} \"{1}\"", ticket.number, ticket.title));

// Add sign-off tags to commit message footer if:
// - there are reviewers on this patchset
// - their review is positive
// - the setting is enabled on repo this patchset is for
if (repository.writeSignoffCommit != null && repository.writeSignoffCommit.length() > 0) {
messageBuilder.append("\n\n");
for (Change change : ticket.getReviews(patchset)) {
// Skip sign-off for negative reviews
if (change.review.score.getValue() < 0) continue;
UserModel ruser = gitblit.getUserModel(change.author);
messageBuilder.append(MessageFormat.format(
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks like it would add a sign-off for someone who gave a negative review but the total score exceeded the minimum requirement. Am I reading that wrong?

Copy link
Author

Choose a reason for hiding this comment

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

Nope, you're correct. Have a look at the follow-up commit, this should fix it.

"{0}: {1} <{2}>\n",
repository.writeSignoffCommit,
ruser.getDisplayName(),
StringUtils.isEmpty(ruser.emailAddress) ? (ruser.username + "@gitblit") : ruser.emailAddress
)
);
}
// Delete extra the line break at the end of the message
messageBuilder.deleteCharAt(messageBuilder.length()-1);
}

// Convert the constructed message to String and continue
String message = messageBuilder.toString();
Ref oldRef = null;
try {
oldRef = getRepository().getRef(ticket.mergeTo);
10 changes: 10 additions & 0 deletions src/main/java/com/gitblit/manager/RepositoryManager.java
Original file line number Diff line number Diff line change
@@ -899,6 +899,8 @@ private RepositoryModel loadRepositoryModel(String repositoryName) {
model.acceptNewPatchsets = getConfig(config, "acceptNewPatchsets", true);
model.acceptNewTickets = getConfig(config, "acceptNewTickets", true);
model.requireApproval = getConfig(config, "requireApproval", settings.getBoolean(Keys.tickets.requireApproval, false));
model.requireScore = getConfig(config, "requireScore", settings.getInteger(Keys.tickets.requireScore, -1));
model.writeSignoffCommit = getConfig(config, "writeSignoffCommit", settings.getString(Keys.tickets.writeSignoffCommit, null));
model.mergeTo = getConfig(config, "mergeTo", null);
model.mergeType = MergeType.fromName(getConfig(config, "mergeType", settings.getString(Keys.tickets.mergeType, null)));
model.useIncrementalPushTags = getConfig(config, "useIncrementalPushTags", false);
@@ -1556,6 +1558,14 @@ public void updateConfiguration(Repository r, RepositoryModel repository) {
// override default
config.setBoolean(Constants.CONFIG_GITBLIT, null, "requireApproval", repository.requireApproval);
}
config.setInt(Constants.CONFIG_GITBLIT, null, "requireScore", repository.requireScore);
if (settings.getString(Keys.tickets.writeSignoffCommit, null) == repository.writeSignoffCommit) {
// use default
config.unset(Constants.CONFIG_GITBLIT, null, "writeSignoffCommit");
} else {
// override default
config.setString(Constants.CONFIG_GITBLIT, null, "writeSignoffCommit", repository.writeSignoffCommit);
}
if (!StringUtils.isEmpty(repository.mergeTo)) {
config.setString(Constants.CONFIG_GITBLIT, null, "mergeTo", repository.mergeTo);
}
4 changes: 3 additions & 1 deletion src/main/java/com/gitblit/models/RepositoryModel.java
Original file line number Diff line number Diff line change
@@ -88,7 +88,9 @@ public class RepositoryModel implements Serializable, Comparable<RepositoryModel
public CommitMessageRenderer commitMessageRenderer;
public boolean acceptNewPatchsets;
public boolean acceptNewTickets;
public boolean requireApproval;
public boolean requireApproval;
public int requireScore;
public String writeSignoffCommit;
public String mergeTo;
public MergeType mergeType;

10 changes: 9 additions & 1 deletion src/main/java/com/gitblit/models/TicketModel.java
Original file line number Diff line number Diff line change
@@ -491,7 +491,6 @@ public List<Change> getReviews(Patchset patchset) {
return new ArrayList<Change>(reviews.values());
}


public boolean isApproved(Patchset patchset) {
if (patchset == null) {
return false;
@@ -512,6 +511,15 @@ public boolean isApproved(Patchset patchset) {
return approved && !vetoed;
}

public int getScore(Patchset patchset) {
int score = 0;

for (Change change : getReviews(patchset)) {
score += change.review.score.getValue();
}
return score;
}

public boolean isVetoed(Patchset patchset) {
if (patchset == null) {
return false;
5 changes: 5 additions & 0 deletions src/main/java/com/gitblit/wicket/GitBlitWebApp.properties
Original file line number Diff line number Diff line change
@@ -556,6 +556,11 @@ gb.acceptNewTickets = allow new tickets
gb.acceptNewTicketsDescription = allow creation of bug, enhancement, task ,etc tickets
gb.requireApproval = require approvals
gb.requireApprovalDescription = patchsets must be approved before merge button is enabled
gb.requireScore = require score
gb.requireScoreDescription = patchsets must be rated this high before merge button is enabled
gb.writeSignoffCommit = write signoff commit
gb.writeSignoffCommitDescription = reviewed patchsets will have lines of the form <em>value: reviewer name &lt;reviewer email&gt;</em> added to their merge commit.\nE.g. if set to <em>Signed-off-by</em>: <code>Signed-off-by: reviewer &lt;reviewer@example.com&gt;</code>. Please see ${commitMessageLink} on the meaning of possible values for this.
gb.writeSignoffCommitLink = Commit Message Conventions in the Linux Kernel
gb.topic = topic
gb.proposalTickets = proposed changes
gb.bugTickets = bugs
11 changes: 10 additions & 1 deletion src/main/java/com/gitblit/wicket/pages/EditRepositoryPage.html
Original file line number Diff line number Diff line change
@@ -122,6 +122,15 @@ <h4><wicket:message key="gb.ticketSettings"></wicket:message></h4>
<div wicket:id="acceptNewPatchsets"></div>
<div wicket:id="acceptNewTickets"></div>
<div wicket:id="requireApproval"></div>
<div wicket:id="requireScore"></div>
<div wicket:id="writeSignoffCommit"></div>
<div>
<wicket:message key="gb.writeSignoffCommitDescription">
<a target="_new" wicket:id="commitMessageLink">
<wicket:message key="gb.writeSignoffCommitLink"></wicket:message>
</a>
</wicket:message>
</div>
<div wicket:id="mergeTo"></div>
<div wicket:id="mergeType"></div>

@@ -200,4 +209,4 @@ <h4><wicket:message key="gb.deleteRepositoryHeader"></wicket:message></h4>
</body>

</wicket:extend>
</html>
</html>
25 changes: 24 additions & 1 deletion src/main/java/com/gitblit/wicket/pages/EditRepositoryPage.java
Original file line number Diff line number Diff line change
@@ -37,6 +37,7 @@
import org.apache.wicket.markup.html.form.CheckBox;
import org.apache.wicket.markup.html.form.ChoiceRenderer;
import org.apache.wicket.markup.html.form.DropDownChoice;
import org.apache.wicket.markup.html.link.ExternalLink;
import org.apache.wicket.markup.html.form.Form;
import org.apache.wicket.markup.html.form.IChoiceRenderer;
import org.apache.wicket.markup.html.form.TextField;
@@ -94,6 +95,8 @@ public class EditRepositoryPage extends RootSubPage {

private IModel<String> mailingLists;

private final static String COMMIT_MESSAGE_LINK = "https://git.wiki.kernel.org/index.php/CommitMessageConventions";

public EditRepositoryPage() {
// create constructor
super();
@@ -411,6 +414,8 @@ protected void onSubmit() {
// do not let the browser pre-populate these fields
form.add(new SimpleAttributeModifier("autocomplete", "off"));

// adds documentation link for localization
form.add(new ExternalLink("commitMessageLink", COMMIT_MESSAGE_LINK));

//
//
@@ -453,7 +458,25 @@ protected void onSubmit() {
getString("gb.requireApproval"),
getString("gb.requireApprovalDescription"),
new PropertyModel<Boolean>(repositoryModel, "requireApproval")));

List<Integer> scores = Arrays.asList(-1, 0, 2, 4, 5, 6, 8);
final boolean allowRequireScore = repositoryModel.requireApproval;
form.add(new ChoiceOption<Integer>("requireScore",
getString("gb.requireScore"),
getString("gb.requireScoreDescription"),
new DropDownChoice<Integer>("choice",
new PropertyModel<Integer>(repositoryModel, "requireScore"),
scores)).setEnabled(allowRequireScore));
List<String> signoffCommitMsgs = Arrays.asList(
null,
"Signed-off-by",
"Reviewed-by",
"Acked-by"
);
form.add(new ChoiceOption<String>("writeSignoffCommit",
getString("gb.writeSignoffCommit"),
new String(),
new PropertyModel<String>(repositoryModel, "writeSignoffCommit"),
signoffCommitMsgs));
form.add(new ChoiceOption<String>("mergeTo",
getString("gb.mergeTo"),
getString("gb.mergeToDescription"),
3 changes: 3 additions & 0 deletions src/main/java/com/gitblit/wicket/pages/TicketPage.java
Original file line number Diff line number Diff line change
@@ -1407,6 +1407,9 @@ protected Component createMergePanel(UserModel user, RepositoryModel repository)
if (repository.requireApproval) {
// repository requires approval
allowMerge = ticket.isOpen() && ticket.isApproved(patchset);
if (repository.requireScore > -1) {
allowMerge &= ticket.getScore(patchset) >= repository.requireScore;
}
} else {
// vetoes are binding
allowMerge = ticket.isOpen() && !ticket.isVetoed(patchset);