-
Notifications
You must be signed in to change notification settings - Fork 85
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
SCANJLIB-244 Log server type info on bootstrap #219
SCANJLIB-244 Log server type info on bootstrap #219
Conversation
57e6ebf
to
c4ba2e4
Compare
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.
Good job!
I like the description in the PR, with the test cases and the follow-up actions. It is helpful for me to understand the scope of the feature as a whole. I also appreciate the description of the test procedure, it makes the review easier.
Do you plan to open the PR for sonar-scanner-cli (and maybe Maven and Gradle) too? Maybe we could do it to take care of this task end-to-end even if we are not the maintainers of the Maven and Gradle scanners, it could be helpful for the JVM squad to just review the changes.
At least, I believe we should open tickets for the 3 scanners and link them to this ticket to make sure the task is not forgotten.
(after discussion, we need a release to do the scanner changes and the release date is not planned yet, so opening tickets is enough)
I'll proceed with testing now.
...rg/sonarsource/scanner/lib/internal/facade/simulation/SimulationScannerEngineFacadeTest.java
Outdated
Show resolved
Hide resolved
lib/src/test/java/org/sonarsource/scanner/lib/internal/util/VersionUtilsTest.java
Show resolved
Hide resolved
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.
Tested successfully!
Communication with sonarcloud.io:
18:34:42.888 INFO Communicating with SonarQube Cloud
And with a local server with different versions:
18:39:51.179 INFO Communicating with SonarQube Community Build 25.1-SNAPSHOT
19:10:08.158 INFO Communicating with SonarQube Server 2025.1-SNAPSHOT
19:22:06.582 INFO Communicating with SonarQube Server 2025.1.0.1234
I added some tasks related to my previous comment in the description though. It is not a blocker to merge this PR, and I think it should be done before closing the ticket.
Also, do you know if/when we plan to release the 3.2 version of this lib?
Thank you for reviewing Claire - I agree and will add the tests before merging, to have it complete in one place. About the tickets in the scanners, yes we can do that after closing this one. And about the release date, I am not sure, since I have merged @henryju's refactoring PR which will introduce some changes in the scanner projects - we should check this with him. |
c4ba2e4
to
77e03b0
Compare
lib/src/main/java/org/sonarsource/scanner/lib/internal/facade/AbstractScannerEngineFacade.java
Outdated
Show resolved
Hide resolved
77e03b0
to
fe6c06b
Compare
Quality Gate passedIssues Measures |
SCANJLIB-244
PR Scope
TODOs:
How to test?
Run the scanner with the local version of sonar-scanner-java-library (3.2-SNAPSHOT)
Tasks