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

Migration Tweaks #6260

Conversation

jolheiser
Copy link
Member

Resolves #6253
Resolves #6254

6253

When migrating, the repo name will attempt to auto-name when the migrate/clone URL is entered. This will only happen if the repo name is empty so that it doesn't overwrite if the user enters something manually first.

6254

Unfortunately, as I described in the issue itself, the sanitization that happens makes the IsErrxxx functions calculate incorrectly, so the handleCreateError was throwing 500s when it shouldn't have been.
I don't particularly like that there is dupe code now, however, so I will likely try to clean that up before readying this PR for review.

Thoughts and suggestions are welcome.

Adds error checking before sanitization in migration

Signed-off-by: jolheiser <[email protected]>
@codecov-io
Copy link

codecov-io commented Mar 7, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@160e7ed). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #6260   +/-   ##
=========================================
  Coverage          ?   41.57%           
=========================================
  Files             ?      446           
  Lines             ?    60848           
  Branches          ?        0           
=========================================
  Hits              ?    25296           
  Misses            ?    32269           
  Partials          ?     3283

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 160e7ed...935f253. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 7, 2019
@jolheiser
Copy link
Member Author

I think this is ready for review.
Since the sanitization of the error made handleCreateError incapable of handling it, I decided it may work best to just implement it separately in Create vs Migrate.

Let me know if you'd prefer an alternate implementation.

@jolheiser jolheiser marked this pull request as ready for review March 7, 2019 19:51
@lafriks lafriks added the type/enhancement An improvement of existing functionality label Mar 9, 2019
@lafriks lafriks added this to the 1.9.0 milestone Mar 9, 2019
@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 May 20, 2019
@lunny
Copy link
Member

lunny commented May 20, 2019

conflicted

Signed-off-by: jolheiser <[email protected]>
@jolheiser
Copy link
Member Author

jolheiser commented May 23, 2019

#6254 was actually fixed in #6290, so this PR really just resolves #6253

Fixed conflict

@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 May 24, 2019
@techknowlogick techknowlogick merged commit 2a8037f into go-gitea:master Jun 4, 2019
@jolheiser jolheiser deleted the 6253_migrate_atuo_name_6254_fix_migrate_error branch July 17, 2019 19:05
jeffliu27 pushed a commit to jeffliu27/gitea that referenced this pull request Jul 18, 2019
* Adds auto-name if repo name is blank
Adds error checking before sanitization in migration

Signed-off-by: jolheiser <[email protected]>

* Changed err from sanitization to a different variable

Signed-off-by: jolheiser <[email protected]>

* Remove handleCreatePost and implement separately

Signed-off-by: jolheiser <[email protected]>

* Make fmt

Signed-off-by: jolheiser <[email protected]>
@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. type/enhancement An improvement of existing functionality
Projects
None yet
8 participants