Skip to content

Conversation

@6543
Copy link
Member

@6543 6543 commented Sep 1, 2020

close #12630

@codecov-commenter
Copy link

codecov-commenter commented Sep 1, 2020

Codecov Report

Merging #12672 into master will decrease coverage by 0.03%.
The diff coverage is 53.70%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #12672      +/-   ##
==========================================
- Coverage   43.22%   43.19%   -0.04%     
==========================================
  Files         651      651              
  Lines       72118    72134      +16     
==========================================
- Hits        31175    31156      -19     
- Misses      35898    35931      +33     
- Partials     5045     5047       +2     
Impacted Files Coverage Δ
modules/structs/repo.go 50.00% <ø> (ø)
modules/convert/utils.go 44.44% <33.33%> (-22.23%) ⬇️
modules/auth/repo_form.go 42.34% <50.00%> (ø)
routers/repo/migrate.go 44.82% <50.00%> (ø)
routers/api/v1/repo/migrate.go 43.60% <53.33%> (-2.13%) ⬇️
models/task.go 40.21% <100.00%> (ø)
modules/migrations/gitea.go 6.70% <100.00%> (-0.38%) ⬇️
modules/repository/repo.go 42.58% <100.00%> (ø)
modules/task/migrate.go 25.80% <100.00%> (ø)
routers/api/v1/api.go 79.08% <100.00%> (ø)
... and 17 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 daefdd1...be8bd63. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 1, 2020
@6543 6543 force-pushed the api-migration-serviceType-string branch from 1cd9dd5 to 6f46bc2 Compare September 1, 2020 13:53
@6543
Copy link
Member Author

6543 commented Sep 1, 2020

@lunny I think I should fix this upstream ... at the moment eaven a between , break things :/

-> Resolved

Copy link
Member

@techknowlogick techknowlogick left a comment

Choose a reason for hiding this comment

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

Testing this PR, when I toggle mirror checkbox it should disable option to migrate issues,PRs, etc..

@6543
Copy link
Member Author

6543 commented Sep 1, 2020

@jolheiser I'll test it again - hope it still works...

@6543 6543 force-pushed the api-migration-serviceType-string branch from a935a4c to 0427ebf Compare September 1, 2020 20:43
@6543
Copy link
Member Author

6543 commented Sep 1, 2020

@jolheiser no it broke "pre selection" to Git

ctx.Data["service"] = structs.PlainGitService

☝️ yes it is no struct but this type has a type-function called Title()

@jolheiser
Copy link
Member

I see, my bad. But now I have another concern.

This PR doesn't actually change the enums to strings, it just provides a way to convert strings into a GitServiceType.

I'm a little worried that this adds unneeded complexity, is it not possible to change the enum to a string, or perhaps just a wrapper for the API?

@6543 6543 changed the title Migration Service Type: change to string enum Migration Service Type: change to string enum for API Sep 1, 2020
@6543 6543 changed the title Migration Service Type: change to string enum for API Migration Service Type: change to string enum for API/Form Sep 1, 2020
@6543
Copy link
Member Author

6543 commented Sep 1, 2020

yes it is doable - will resullt in dublicating the struct ...

@6543 6543 changed the title Migration Service Type: change to string enum for API/Form WIP: Migration Service Type: change to string enum for API Sep 1, 2020
@6543 6543 force-pushed the api-migration-serviceType-string branch from 0427ebf to b7ea054 Compare September 1, 2020 22:07
@6543 6543 changed the title WIP: Migration Service Type: change to string enum for API Migration Service Type: change to string enum for API Sep 1, 2020
@6543 6543 changed the title Migration Service Type: change to string enum for API [API] Migration: Change ServiceType String Sep 1, 2020
@6543
Copy link
Member Author

6543 commented Sep 1, 2020

@jolheiser Done

@6543 6543 requested a review from techknowlogick September 1, 2020 22:44
@6543
Copy link
Member Author

6543 commented Sep 1, 2020

I included the refactor because I think it is realy needed in this pull ...

Refactor:

  • the Options for WebUI: modules/auth/repo_form.go
  • the Options for API: modules/structs/repo.go
  • the option struct after validation & supplementing for internal prossessing: modules/migrations/base/options.go

@6543
Copy link
Member Author

6543 commented Sep 2, 2020

@techknowlogick If this is all you can find then I'm totaly fine with it 🙃

@6543 6543 requested a review from techknowlogick September 3, 2020 01:29
@6543 6543 added this to the 1.13.0 milestone Sep 4, 2020
@6543 6543 added modifies/api This PR adds API routes or modifies them type/refactoring Existing code has been cleaned up. There should be no new functionality. labels Sep 4, 2020
@lunny
Copy link
Member

lunny commented Sep 10, 2020

Please resolve the conflicts.

@6543
Copy link
Member Author

6543 commented Sep 10, 2020

@lunny done

@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 10, 2020
@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 10, 2020
@6543
Copy link
Member Author

6543 commented Sep 10, 2020

@zeripath added doc what purpose each struct has

@techknowlogick can you either dismiss review or have a look at it?

@techknowlogick techknowlogick dismissed their stale review September 10, 2020 21:10

dismiss my review

@6543
Copy link
Member Author

6543 commented Sep 10, 2020

thanks @ techknowlogick

@zeripath zeripath merged commit fd60ebf into go-gitea:master Sep 10, 2020
@6543 6543 deleted the api-migration-serviceType-string branch September 10, 2020 23:16
@6543
Copy link
Member Author

6543 commented Sep 10, 2020

@zeripath thanks :)

zeripath pushed a commit that referenced this pull request Oct 12, 2020
@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. modifies/api This PR adds API routes or modifies them 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.

[API] Migration Service Type should be string enum

8 participants