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

Some refactor on git diff and ignore getting commit information failed on migrating pull request review comments #9996

Merged
merged 5 commits into from Jan 28, 2020

Conversation

lunny
Copy link
Member

@lunny lunny commented Jan 26, 2020

When migrating a repository from github with pull requests, if the pull request review comments refs an outdate commit which is not yet on this repository any more. We should ignore that commits.

This pull request also moved some git diff functions from package gitdiff to git.

@lunny lunny added type/enhancement An improvement of existing functionality skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. labels Jan 26, 2020
@lunny lunny added this to the 1.12.0 milestone Jan 26, 2020
@lunny lunny force-pushed the lunny/ignore_migrate_review_commet branch from eb58f8e to cf6a929 Compare January 26, 2020 12:18
@codecov-io
Copy link

codecov-io commented Jan 26, 2020

Codecov Report

Merging #9996 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9996      +/-   ##
==========================================
+ Coverage   42.26%   42.26%   +<.01%     
==========================================
  Files         611      611              
  Lines       80391    80391              
==========================================
+ Hits        33976    33978       +2     
+ Misses      42237    42235       -2     
  Partials     4178     4178
Impacted Files Coverage Δ
modules/log/event.go 64.61% <0%> (-1.03%) ⬇️
models/repo.go 49.66% <0%> (-0.14%) ⬇️
services/pull/patch.go 67.92% <0%> (ø) ⬆️
modules/queue/workerpool.go 42.48% <0%> (+1.28%) ⬆️
modules/process/manager.go 78.31% <0%> (+3.61%) ⬆️

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 733a395...1e187c2. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 26, 2020
@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 Jan 27, 2020
@6543
Copy link
Member

6543 commented Jan 27, 2020

Beside modules/migrations/gitea.go it is a refactor

@lunny lunny added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Jan 27, 2020
@zeripath
Copy link
Contributor

@lunny it's hard to see what you've fixed and what's been moved - I wonder could you split the commits in two - one doing the move and one doing the fix?

Copy link
Member

@guillep2k guillep2k left a comment

Choose a reason for hiding this comment

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

Just a couple of nits.

modules/git/diff.go Show resolved Hide resolved
modules/migrations/gitea.go Outdated Show resolved Hide resolved
@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 Jan 27, 2020
@lunny lunny force-pushed the lunny/ignore_migrate_review_commet branch from cf6a929 to 733a395 Compare January 28, 2020 02:15
@zeripath zeripath merged commit e8860ef into go-gitea:master Jan 28, 2020
@lunny lunny deleted the lunny/ignore_migrate_review_commet branch January 28, 2020 09:10
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. type/enhancement An improvement of existing functionality type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants