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

Move git diff codes from models to services/gitdiff #7889

Merged
merged 7 commits into from Sep 6, 2019

Conversation

@lunny
Copy link
Member

commented Aug 16, 2019

As title.

@lunny lunny added the kind/refactor label Aug 16, 2019

@lunny lunny added this to the 1.10.0 milestone Aug 16, 2019

@codecov-io

This comment has been minimized.

Copy link

commented Aug 16, 2019

Codecov Report

Merging #7889 into master will decrease coverage by <.01%.
The diff coverage is 7.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7889      +/-   ##
==========================================
- Coverage   41.59%   41.58%   -0.01%     
==========================================
  Files         479      480       +1     
  Lines       64106    64108       +2     
==========================================
- Hits        26662    26659       -3     
- Misses      33986    33993       +7     
+ Partials     3458     3456       -2
Impacted Files Coverage Δ
modules/templates/helper.go 48.46% <ø> (ø) ⬆️
models/issue_comment.go 50.97% <ø> (+3.41%) ⬆️
models/models.go 61.93% <0%> (-0.81%) ⬇️
routers/repo/commit.go 29.03% <0%> (ø) ⬆️
routers/repo/pull_review.go 0% <0%> (ø) ⬆️
services/comments/comments.go 0% <0%> (ø)
routers/repo/pull.go 31.61% <0%> (ø) ⬆️
modules/repofiles/temp_repo.go 70.5% <100%> (ø) ⬆️
modules/repofiles/diff.go 60.86% <100%> (ø) ⬆️
routers/repo/compare.go 53.84% <100%> (ø) ⬆️
... and 7 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 ab0f378...a805dde. Read the comment docs.

@typeless
Copy link
Contributor

left a comment

What is the main difference in purposes between modules and services directories?

"code.gitea.io/gitea/services/gitdiff"
)

// CreateCodeComment creates a plain code comment at the specified line / path

This comment has been minimized.

Copy link
@typeless

typeless Aug 19, 2019

Contributor

CodeComment is a bit too general. It could mean comments in the source code at first glance.
Maybe ReviewComment if there are no other types of comments?
If renaming it would involve extensive changes beyond this PR, no need to bother now.

This comment has been minimized.

Copy link
@lunny

lunny Aug 19, 2019

Author Member

I would like to leave it for another PR.

This comment has been minimized.

Copy link
@typeless

typeless Aug 19, 2019

Contributor

No problem.

This comment has been minimized.

Copy link
@typeless

typeless Aug 19, 2019

Contributor

@lunny By the way, after realizing it's already there and this is just a refactor. I think it is not worth the effort to change the name at this point.


// FIXME validate treePath
// Get latest commit referencing the commented line
// No need for get commit for base branch changes

This comment has been minimized.

Copy link
@typeless

typeless Aug 19, 2019

Contributor

'No need to get ... or 'No need of getting...'?

This comment has been minimized.

Copy link
@lunny

lunny Aug 19, 2019

Author Member

Just move them and haven't see it's correct.

}
}

// Only fetch diff if comment is review comment

This comment has been minimized.

Copy link
@typeless

typeless Aug 19, 2019

Contributor

Under what circumstances reviewID would be 0?

This comment has been minimized.

Copy link
@lunny

lunny Aug 19, 2019

Author Member

Just move them and haven't see it's correct.

@lunny

This comment has been minimized.

Copy link
Member Author

commented Aug 19, 2019

services will depend on modes/modules and be dependent by routers/commands.

@GiteaBot GiteaBot added lgtm/need 1 and removed lgtm/need 2 labels Aug 19, 2019

@lunny lunny force-pushed the lunny:lunny/diff branch from 7393a05 to afcca9a Aug 26, 2019

lunny added 4 commits Aug 16, 2019

@lunny lunny force-pushed the lunny:lunny/diff branch from afcca9a to a805dde Aug 27, 2019

@GiteaBot GiteaBot added lgtm/done and removed lgtm/need 1 labels Sep 3, 2019

lunny and others added 2 commits Sep 4, 2019
@techknowlogick

This comment has been minimized.

Copy link
Member

commented Sep 6, 2019

ping LG-TM bot

@techknowlogick techknowlogick merged commit c03d75f into go-gitea:master Sep 6, 2019

2 checks passed

approvals/lgtm this commit looks good
continuous-integration/drone/pr Build is passing
Details

@lunny lunny deleted the lunny:lunny/diff branch Sep 6, 2019

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