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 to migrate from gogs #14342

Merged
merged 54 commits into from
Jan 21, 2021
Merged

Add support to migrate from gogs #14342

merged 54 commits into from
Jan 21, 2021

Conversation

6543
Copy link
Member

@6543 6543 commented Jan 14, 2021 •
edited
Loading

Add support to migrate gogs:

  • issues
  • comments
  • labels
  • milestones
  • wiki

6543
6543 commented Jan 14, 2021
View reviewed changes
modules/migrations/gogs_test.go Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 14, 2021
@6543 6543 added type/feature Completely new functionality. Can only be merged if feature freeze is not active. topic/repo-migration Migrate repos from other platforms to Gitea, or from Gitea to them labels Jan 14, 2021
6543
6543 commented Jan 14, 2021
View reviewed changes
modules/migrations/migrate.go Outdated Show resolved Hide resolved
@6543
Copy link
Member Author

6543 commented Jan 15, 2021

f8635eb is a refactor and can be moved into next pull if it makes this pull hard to review or block it ...

@6543 6543 force-pushed the lunny/downloader_gogs branch from de3955e to 24230d4 Compare January 15, 2021 00:55
@6543 6543 force-pushed the lunny/downloader_gogs branch from 24230d4 to 7518582 Compare January 15, 2021 01:25
@lunny lunny mentioned this pull request Jan 15, 2021
Closed
@6543 6543 added this to the 1.14.0 milestone Jan 15, 2021
@6543 6543 mentioned this pull request Jan 15, 2021
Open
24 tasks
@6543
Copy link
Member Author

6543 commented Jan 17, 2021

OK 2 thoughts:

1. Without PR migration this is somewhat limited - Gogs users will immediately complain about this lack.

2. We're adding a load of gogs specific code here - are we happy with the safety of this library? We should look carefully at this.

It may be that we should add a build tag to build this in or not.

  1. We do not have mouch of a chance since ther is no api, and so gogs user should complain on the gogs issue tracker about it .... this could potentialy create one issue here too but i cam live with it ... (and i have a follow up pull with an hack to somehow get some more bits ;) )
  2. This is the same code as our go-sdk just not that enhanced & maintained ... if there are more concerns I'll create a fork specific for our migrator ... and strip down functions ...

@6543 6543 mentioned this pull request Jan 19, 2021
Closed
@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 21, 2021
zeripath
zeripath approved these changes Jan 21, 2021
View reviewed changes
Copy link
Contributor

@zeripath zeripath left a comment

Choose a reason for hiding this comment

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

Approving but we need the following extensions:

  • We need to get pulls migrated too.
  • We need to allow server managers to decide whether or not to enable all the different types of migrator. .

@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 21, 2021
@6543 6543 merged commit 81c833d into go-gitea:master Jan 21, 2021
@6543 6543 deleted the lunny/downloader_gogs branch January 21, 2021 19:34
@go-gitea go-gitea locked and limited conversation to collaborators Mar 11, 2021
wxiaoguang
wxiaoguang reviewed Jun 30, 2022
View reviewed changes
modules/migrations/base/error.go

// IsErrNotSupported checks if an error is an ErrNotSupported
func IsErrNotSupported(err error) bool {
_, ok := err.(ErrNotSupported)
Copy link
Contributor

@wxiaoguang wxiaoguang Jun 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Seems incorrect here, because the NullDownloader code returns pointer: return nil, &ErrNotSupported{}

Or here is correct but NullDownloader does wrong 😂

Copy link
Contributor

Choose a reason for hiding this comment

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

Will be fixed in

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@wxiaoguang wxiaoguang wxiaoguang left review comments

@zeripath zeripath zeripath 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. topic/repo-migration Migrate repos from other platforms to Gitea, or from Gitea to them type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Milestone
1.14.0
Development

Successfully merging this pull request may close these issues.
7 participants
@6543 @Cat7373 @codecov-io @zeripath @lunny @wxiaoguang @GiteaBot