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

Add images base URL property #150

Merged
merged 3 commits into from
May 17, 2020

Conversation

Brialius
Copy link
Contributor

@Brialius Brialius commented Apr 1, 2020

Fix #130

@Brialius
Copy link
Contributor Author

Brialius commented Apr 1, 2020

@don-vip please take a look

@Brialius
Copy link
Contributor Author

Brialius commented Apr 1, 2020

Copy link
Contributor

@don-vip don-vip left a comment

Choose a reason for hiding this comment

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

Please also document the property in README. I removed it, it must be restored.

@Brialius
Copy link
Contributor Author

Brialius commented Apr 1, 2020

Please also document the property in README. I removed it, it must be restored.

Thanks for review!

Instead of previous approach I made new property configurable via server UI with appropriate field description: https://github.com/mc1arke/sonarqube-community-branch-plugin/pull/150/files#diff-6b77d55ef12cf09f58f256a700f17effR93-R97

But you are right, maybe an explanation in the README could be useful to get the clue about new property usage.

@4n4n4s
Copy link
Contributor

4n4n4s commented Apr 1, 2020

@Brialius
Copy link
Contributor Author

Brialius commented Apr 1, 2020

Hi @Brialius @don-vip
so adding https://raw.githubusercontent.com/SonarSource/sonarcloud-github-static-resources/gh-pages/v2/ as base URL should work fine right?

Hey @4n4n4s
I suppose any storage with the same folders structure and images inside should work fine.

Pls notice that there is no trailing slash in base URL

@4n4n4s
Copy link
Contributor

4n4n4s commented Apr 1, 2020

Pls notice that there is no trailing slash in base URL

Maybe we should remove it in case someone configures it with a slash to prevent misconfiguration

Add tests for getBaseImageUrl()
@Brialius
Copy link
Contributor Author

Brialius commented Apr 1, 2020

Pls notice that there is no trailing slash in base URL

Maybe we should remove it in case someone configures it with a slash to prevent misconfiguration

@4n4n4s
fixed

@Brialius
Copy link
Contributor Author

Brialius commented Apr 11, 2020

@don-vip property description is added to README

don-vip
don-vip previously approved these changes Apr 11, 2020
@Brialius
Copy link
Contributor Author

@mc1arke could you please take a look at this PR?

Copy link
Owner

@mc1arke mc1arke left a comment

Choose a reason for hiding this comment

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

Thanks for the great contribution. I'm generally happy with your changes, but want a discussion on the points I've raised before merging.

@mc1arke mc1arke closed this Apr 26, 2020
@mc1arke mc1arke reopened this Apr 26, 2020
@Brialius Brialius force-pushed the Add-image-base-url-property branch from ef82deb to 9dccee6 Compare April 27, 2020 08:51
@mc1arke mc1arke merged commit f09f72c into mc1arke:master May 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[GitHub][PR Decoration] Private Sonarqube server static images unavailable
4 participants