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.

Sign up for GitHub

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

Jump to bottom

Add possibility to record branch information in an issue #780

Merged
merged 1 commit into from
Aug 24, 2017
Merged

Add possibility to record branch information in an issue #780

merged 1 commit into from
Aug 24, 2017

Conversation

gzsombor
Copy link
Contributor

This patch add a new field to the 'issue' entity, named 'branch' with the necessary UI to specify which branch this issue is referring.
Searching, filtering and editing is not yet implemented, so the branch can only specified when creating the issue

@andreynering andreynering added type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/proposal The new feature has not been accepted yet but needs to be discussed first. labels Jan 28, 2017
@andreynering andreynering added this to the 1.2.0 milestone Jan 28, 2017
andreynering
andreynering reviewed Feb 4, 2017
View reviewed changes
Copy link
Contributor

@andreynering andreynering left a comment

Choose a reason for hiding this comment

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

I'd like to hear the @go-gitea/owners opinion on whether we will accept or not this feature.

options/locale/locale_en-US.ini Outdated
@@ -531,6 +531,7 @@ issues.new.closed_milestone = Closed Milestones
issues.new.assignee = Assignee
issues.new.clear_assignee = Clear assignee
issues.new.no_assignee = No assignee
issues.noBranch = No Branch Specified
Copy link
Contributor

Choose a reason for hiding this comment

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

noBranch -> no_branch

templates/repo/issue/new_form.tmpl Outdated
<input id="branch_selector" name="branch" type="hidden" value="{{.branch}}">
<div class="ui floating filter select-branch dropdown" data-no-results="{{.i18n.Tr "repo.pulls.no_results"}}">
<div class="ui basic small button">
<span class="text branch-name">{{if .branch}}{{.branch}}{{else}}{{.i18n.Tr "repo.issues.noBranch"}}{{end}}</span>
Copy link
Contributor

Choose a reason for hiding this comment

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

noBranch -> no_branch

templates/repo/issue/view_content.tmpl Outdated
@@ -253,6 +253,26 @@

<div class="four wide column">
<div class="ui segment metas">
<div class="ui disabled floating filter select-branch dropdown" data-no-results="{{.i18n.Tr "repo.pulls.no_results"}}">
<div class="ui basic small button">
<span class="text branch-name">{{if .Issue.Branch}}{{.Issue.Branch}}{{else}}{{.i18n.Tr "repo.issues.noBranch"}}{{end}}</span>
Copy link
Contributor

Choose a reason for hiding this comment

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

noBranch -> no_branch

@lunny
Copy link
Member

lunny commented Feb 4, 2017

I think this PR is good point, but maybe also need tag or commit? Of course, put it in v1.2 is a correct decision. @andreynering

@tboerger tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 4, 2017
@gzsombor
Copy link
Contributor Author

gzsombor commented Feb 5, 2017

I've implemented the requested changes from noBranch to no_branch
As the branch field is just a string, theoretically it could also contain a tag or a commit, if the frontend component is changed

@bkcsoft
Copy link
Member

bkcsoft commented Feb 6, 2017

@lunny better to track the branch only. GitLab also has this feature, very useful when an issue has many PRs 👍

@tboerger
Copy link
Member

I like this feature, but I would like to be able to also select a tag. Pretty good for bug reports for deployed applications.

@gzsombor
Copy link
Contributor Author

Do I need to do anything with it to merge ?
I guess, it's not really hard to make this field configurable - so on a repository level, the admin can say, if it's needs this field, it should contain branch, or tag, or both. But that should go to a different commit :)

@lunny
Copy link
Member

lunny commented Feb 28, 2017

I will really review this after 1.1 released.

@lunny
Copy link
Member

lunny commented Mar 10, 2017

@gzsombor could you add tag selection also?

@lunny
Copy link
Member

lunny commented Apr 20, 2017

conflict

@lunny lunny modified the milestones: 1.3.0, 1.2.0 Apr 27, 2017
@gzsombor
Copy link
Contributor Author

Patch is updated with tag selection

@gzsombor
Copy link
Contributor Author

What do you think?

@lunny
Copy link
Member

lunny commented Aug 21, 2017

@gzsombor I will review this again.

bkcsoft
bkcsoft reviewed Aug 21, 2017
View reviewed changes
Copy link
Member

@bkcsoft bkcsoft left a comment

Choose a reason for hiding this comment

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

Just semanital cleanup, otherwise LGTM

models/issue.go Outdated
@@ -49,6 +49,7 @@ type Issue struct {
IsPull bool `xorm:"INDEX"` // Indicates whether is a pull request or not.
PullRequest *PullRequest `xorm:"-"`
NumComments int
Branch string
Copy link
Member

Choose a reason for hiding this comment

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

Maybe call this Ref instead, since we can track both Branches and Tags?

modules/auth/repo_form.go Outdated
@@ -194,6 +194,7 @@ func (f *NewSlackHookForm) Validate(ctx *macaron.Context, errs binding.Errors) b
type CreateIssueForm struct {
Title string `binding:"Required;MaxSize(255)"`
LabelIDs string `form:"label_ids"`
Branch string `form:"branch"`
Copy link
Member

Choose a reason for hiding this comment

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

Same here, Ref

options/locale/locale_en-US.ini Outdated
@@ -600,6 +600,7 @@ issues.new.closed_milestone = Closed Milestones
issues.new.assignee = Assignee
issues.new.clear_assignee = Clear assignee
issues.new.no_assignee = No assignee
issues.no_branch = No Branch Specified
Copy link
Member

Choose a reason for hiding this comment

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

issues.no_ref = No Branch/Tag Specified

@gzsombor
Copy link
Contributor Author

I've fixed the issues

@bkcsoft
Copy link
Member

bkcsoft commented Aug 24, 2017

LGTM

@tboerger tboerger 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 24, 2017
@lafriks
Copy link
Member

lafriks commented Aug 24, 2017

LGTM

@tboerger tboerger 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 24, 2017
@lafriks lafriks merged commit da230a2 into go-gitea:master Aug 24, 2017
@thehowl thehowl mentioned this pull request Mar 3, 2018
Closed
@go-gitea go-gitea locked and limited conversation to collaborators Nov 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bkcsoft bkcsoft bkcsoft left review comments

@andreynering andreynering andreynering left review comments

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
Milestone
1.3.0
Development

Successfully merging this pull request may close these issues.
6 participants
@gzsombor @lunny @bkcsoft @tboerger @lafriks @andreynering