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 support for forking single branch #25821

Merged
merged 13 commits into from
Sep 29, 2023
Merged

Add support for forking single branch #25821

merged 13 commits into from
Sep 29, 2023

Conversation

dsseng
Copy link
Contributor

@dsseng dsseng commented Jul 11, 2023 •
edited by lunny
Loading

Fixes #25117

Add UI for choosing branch to fork

Change default branch on single-branch forks

image

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 11, 2023
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 11, 2023
dsseng
dsseng commented Jul 11, 2023
View reviewed changes
services/repository/fork.go Outdated Show resolved Hide resolved
@dsseng
Copy link
Contributor Author

dsseng commented Jul 11, 2023

Extra idea: maybe we should support the same option for migrations? It's maybe even more useful in such case when people migrate (essentially fork a repo to their instance) for development purpose

@yp05327
Copy link
Contributor

yp05327 commented Jul 11, 2023

I understand the point of fork arbitrary branch, but list all branches at one time may cause performance problem.
Loading branches page by page is better, but I'm not sure whether we have this feature. 🤔

@dsseng
Add support for forking single branch
0d4460d 
Fixes go-gitea#25117

Add UI for choosing branch to fork

Change default branch on single-branch forks
@dsseng
Copy link
Contributor Author

dsseng commented Jul 11, 2023

I understand the point of fork arbitrary branch, but list all branches at one time may cause performance problem. Loading branches page by page is better, but I'm not sure whether we have this feature. thinking

Other places where branch list is used do render it in once. Listing branches and putting them into a template shouldn't hog the performance a lot

@denyskon denyskon added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Sep 11, 2023
@denyskon denyskon added this to the 1.22.0 milestone Sep 11, 2023
@denyskon
Copy link
Member

@sh7dm Could you please resolve conflicts? I totally missed this PR, but it looks uncomplicated to review. Unfortunately we're already in feature freeze for 1.21 though...

@denyskon denyskon mentioned this pull request Sep 22, 2023
Closed
@denyskon denyskon added the giteabot/update-branch Hint for the bot that it should update a PR with the latest state on main label Sep 24, 2023
@GiteaBot GiteaBot removed the giteabot/update-branch Hint for the bot that it should update a PR with the latest state on main label Sep 24, 2023
denyskon
denyskon approved these changes Sep 24, 2023
View reviewed changes
Copy link
Member

@denyskon denyskon left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I'd like for you to use the new translation structure (see my suggestions), but in general, this PR seems to look good

templates/repo/pulls/fork.tmpl Outdated Show resolved Hide resolved
templates/repo/pulls/fork.tmpl Outdated Show resolved Hide resolved
templates/repo/pulls/fork.tmpl Outdated Show resolved Hide resolved
templates/repo/pulls/fork.tmpl Outdated Show resolved Hide resolved
templates/repo/pulls/fork.tmpl Outdated Show resolved Hide resolved
@GiteaBot GiteaBot 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 Sep 24, 2023
@dsseng @denyskon
Apply suggestions from code review
f1f0c68 
Co-authored-by: Denys Konovalov <kontakt@denyskon.de>
@lunny
Copy link
Member

lunny commented Sep 27, 2023

Could you paste a screenshot?

@dsseng
Copy link
Contributor Author

dsseng commented Sep 27, 2023

image

@dsseng @lunny
Refactor git clone argument generation
fd5603d 
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
lunny
lunny reviewed Sep 27, 2023
View reviewed changes
routers/web/repo/pull.go Outdated Show resolved Hide resolved
@denyskon denyskon requested a review from lunny September 27, 2023 20:11
@delvh
Copy link
Member

delvh commented Sep 28, 2023

Alternative design idea:
Offer a radio group with the following possibilities:
- all
- default branch
- custom
Where custom opens a checkbox group of all available branches.
I think this improves both the functionality as well as being more usable, as the two most often used values are easy to find/discern.

@GiteaBot GiteaBot 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 Sep 28, 2023
@lunny
Copy link
Member

lunny commented Sep 28, 2023

Alternative design idea: Offer a radio group with the following possibilities: - all - default branch - custom Where custom opens a checkbox group of all available branches. I think this improves both the functionality as well as being more usable, as the two most often used values are easy to find/discern.

I think it's good idea but could be another PR. At least, this PR allow we fork with a single branch.

@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Sep 28, 2023
@lunny lunny merged commit 5e02e3b into go-gitea:main Sep 29, 2023
26 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Sep 29, 2023
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Dec 28, 2023
@lunny lunny added the type/changelog Adds the changelog for a new Gitea version label Mar 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sad75 sad75 sad75 left review comments

@wxiaoguang wxiaoguang wxiaoguang left review comments

@denyskon denyskon denyskon approved these changes

@lunny lunny lunny approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. type/changelog Adds the changelog for a new Gitea version type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Milestone
1.22.0
Development

Successfully merging this pull request may close these issues.

Fork using only a single branch or tag
8 participants
@dsseng @yp05327 @denyskon @lunny @delvh @wxiaoguang @sad75 @GiteaBot