-
Notifications
You must be signed in to change notification settings - Fork 370
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
Don't pull avatar if remote GitHub Enterprise uses private mode. #532
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this would be clearer if exceptions where not used to control the flow - especially for the majority use case.
src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator.java
Outdated
Show resolved
Hide resolved
src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator.java
Outdated
Show resolved
Hide resolved
} catch (MalformedURLException e) { | ||
// Ignored | ||
} catch (IOException e) { | ||
if (e.getMessage().contains("private mode enabled")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator.java
Outdated
Show resolved
Hide resolved
Looking at this again - I do not think this is even the correct fix and only fixes half the issue (if the other half is fixed then this is not needed at all). Basically the image is not loaded so the browser is falling back to what it should do - the "alt text" (which is "GitHub Organisation") if image loading is disabled you will see exactly the same issue against the public github.com (shown here for ci.jenkins.io) The issue would seem to be that the alt text is too wide for the column (and the column is not resizing) to accommodate it. |
Co-authored-by: James Nord <[email protected]>
src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator.java
Outdated
Show resolved
Hide resolved
Co-authored-by: James Nord <[email protected]>
src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator.java
Outdated
Show resolved
Hide resolved
…bSCMNavigator.java
Looks like there hasn't been an incremental generated since the first commit. Apparently they are just |
Description
When defining a GitHub organization pointing at a Github Enterprise instance running in private mode, the avatar URL can't be accessed directly. This is causing a broken icon to be displayed as well as layout problems in list view in recent Jenkins versions.
This patch detects if the remote instance is in private mode and fallbacks to the standard github icon in that case.
The organization folder needs to be saved after upgrading to apply the new logic.
Before
![Capture d’écran 2022-04-06 à 11 58 39](https://user-images.githubusercontent.com/171459/161979279-7ca22ddc-82b0-439c-a0d5-cb292830dc34.png)
After
![Capture d’écran 2022-04-06 à 14 59 49](https://user-images.githubusercontent.com/171459/161980234-a67c41d8-af18-4283-80e0-d9d3bf36008d.png)
Submitter checklist
Reviewer checklist
Documentation changes
Users/aliases to notify