Refactor PluginManager advanced page handling a bit #2703

Merged
merged 1 commit into from Jan 8, 2017

Conversation

3 participants
@olivergondza
Member

olivergondza commented Jan 4, 2017

  • Redirect back to advanced page after saving site url.
  • Reduced the scope of catch(Exception) block.
- } catch (IOException e) {
- throw e;
- } catch (Exception e) {// grrr. fileItem.write throws this
+ } catch (FileUploadException e) {

This comment has been minimized.

@oleg-nenashev

oleg-nenashev Jan 6, 2017

Member

It may impact RuntimeExceptions if they somehow happen within the code

@oleg-nenashev

oleg-nenashev Jan 6, 2017

Member

It may impact RuntimeExceptions if they somehow happen within the code

This comment has been minimized.

@daniel-beck

daniel-beck Jan 7, 2017

Member

They probably shouldn't have been caught in this catch-all net. As this is just a Stapler call I wouldn't worry too much about it.

@daniel-beck

daniel-beck Jan 7, 2017

Member

They probably shouldn't have been caught in this catch-all net. As this is just a Stapler call I wouldn't worry too much about it.

This comment has been minimized.

@oleg-nenashev

oleg-nenashev Jan 7, 2017

Member

As this is just a Stapler call I wouldn't worry too much about it.

OH oh, OK

@oleg-nenashev

oleg-nenashev Jan 7, 2017

Member

As this is just a Stapler call I wouldn't worry too much about it.

OH oh, OK

@@ -1524,10 +1525,9 @@ public HttpResponse doSiteConfigure(@QueryParameter String site) throws IOExcept
}
sites.add(new UpdateSite(UpdateCenter.ID_DEFAULT, site));
- return HttpResponses.redirectToContextRoot();
+ return new HttpRedirect("advanced");

This comment has been minimized.

@daniel-beck

daniel-beck Jan 8, 2017

Member

I always hated this 👍

@daniel-beck

daniel-beck Jan 8, 2017

Member

I always hated this 👍

This comment has been minimized.

@oleg-nenashev

🚢 🇮🇹

@oleg-nenashev oleg-nenashev merged commit ef66651 into jenkinsci:master Jan 8, 2017

2 checks passed

Jenkins This pull request looks good
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment