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.

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

Change target branch for pull request #6488

Open
wants to merge 2 commits into
base: master
from

Conversation

6 participants
@saitho
Copy link
Contributor

commented Apr 2, 2019

This pull request aims to implement the feature in #6377 .

I'm new to Golang. So far the feature works, however there is still some work to do:

  • Resolve build issues (linting etc)
  • Reimplement webhooks that are executed when target branch is changed
  • Read and adhere to Contribution guidelines
  • Squash commits into one commit that follows the existing commit style of the project

@saitho saitho changed the title WIP: (feat) Change target branch for pull request WIP: (feat) Change target branch for pull request (#6377) Apr 2, 2019

Show resolved Hide resolved models/issue.go Outdated

@GiteaBot GiteaBot added the lgtm/need 2 label Apr 2, 2019

Show resolved Hide resolved options/locale/locale_bg-BG.ini Outdated
@codecov-io

This comment has been minimized.

Copy link

commented Apr 2, 2019

Codecov Report

Merging #6488 into master will decrease coverage by 0.03%.
The diff coverage is 21.1%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6488      +/-   ##
==========================================
- Coverage   40.88%   40.85%   -0.04%     
==========================================
  Files         420      421       +1     
  Lines       57453    57561     +108     
==========================================
+ Hits        23490    23516      +26     
- Misses      30837    30919      +82     
  Partials     3126     3126
Impacted Files Coverage Δ
models/migrations/migrations.go 1.52% <ø> (ø) ⬆️
models/pull.go 48.88% <0%> (-1.67%) ⬇️
models/migrations/v85.go 0% <0%> (ø)
routers/routes/routes.go 82.36% <100%> (+0.06%) ⬆️
models/issue_comment.go 46.51% <18.18%> (-0.4%) ⬇️
routers/repo/pull.go 35.75% <3.7%> (-0.92%) ⬇️
routers/repo/issue.go 37.81% <80%> (+0.71%) ⬆️
models/repo_list.go 66.84% <0%> (-1.06%) ⬇️
models/gpg_key.go 54.72% <0%> (+0.83%) ⬆️
... and 1 more

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 6cb127d...1c01294. Read the comment docs.

@saitho saitho force-pushed the saitho:feature/choose_pullreq_target branch from 38a167a to e4e9971 Apr 2, 2019

@techknowlogick
Copy link
Member

left a comment

Please add migration for changes to structs

@saitho saitho force-pushed the saitho:feature/choose_pullreq_target branch from e4e9971 to 6168c9b Apr 2, 2019

@saitho saitho changed the title WIP: (feat) Change target branch for pull request (#6377) Change target branch for pull request (#6377) Apr 2, 2019

@saitho saitho force-pushed the saitho:feature/choose_pullreq_target branch from 6168c9b to d38e934 Apr 2, 2019

Adds functionality to change target branch of created pull requests
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>

@saitho saitho force-pushed the saitho:feature/choose_pullreq_target branch from d38e934 to f985e67 Apr 2, 2019

@saitho

This comment has been minimized.

Copy link
Contributor Author

commented Apr 2, 2019

I'm done for now. :) Please have a look at it again @techknowlogick @lunny :)

The only thing that bugs me is that pull requests are nested inside issues...

After clicking the "Edit" button on a pull request page the user is able to change the title (as before) and select a new target branch from a select box. Since the title belongs to issues and the target branch belongs to a pull request there will be 2 HTTP requests. One setting the title via the existing issue endpoint and one setting the target branch via a new pull request endpoint. Which is also why two web hooks will be fired. :(

@lafriks lafriks added this to the 1.9.0 milestone Apr 8, 2019

@saitho

This comment has been minimized.

Copy link
Contributor Author

commented Apr 20, 2019

What needs to be done in order to merge this? :)
(Apart from resolving the merge conflicts on the migrations files, obviously ^^)

@techknowlogick

This comment has been minimized.

Copy link
Member

commented Apr 20, 2019

Im currently traveling so I can't review right now, but I'll dismiss my review so it doesn't block this PR.

so this doesn't block the PR

@lunny

This comment has been minimized.

Copy link
Member

commented Apr 20, 2019

please resolve the file conflicting.

@lunny

This comment has been minimized.

Copy link
Member

commented Apr 20, 2019

I think there are still some work need to do when change the branch of pull request. It should affect many aspects.

@saitho saitho changed the title Change target branch for pull request (#6377) Change target branch for pull request Apr 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.