Changed deprecated projectKey parameters while fetching Quality Profile #899
Conversation
mickael-caro-sonarsource
force-pushed
the
mc/804
branch
from
0eef276 to
0ea8252
Compare
mickael-caro-sonarsource
requested review from
andrei-epure-sonarsource and
duncanp-lseg
There was a problem hiding this comment.
The change looks ok to me. It's a breaking change, so there are the same questions as for ProjectGuid PR i.e. what's the minimum supported version of SQ, how will the user know, and what will their experience be if they use it against an old version of SQ?
As far as the UX goes, I think the web API call won't fail since the parameter is optional, and I don't think SQ complains if you give it an unknown parameter. Instead, the search will return a list of multiple projects. That might mean the user ends up with a random QP from a different project.
| @@ -57,7 +57,7 @@ public SonarWebService(IDownloader downloader, string server, ILogger logger) | |||
| { | |||
| var projectId = GetProjectIdentifier(projectKey, projectBranch); | |||
|
|
|||
| var ws = await AddOrganization(GetUrl("/api/qualityprofiles/search?projectKey={0}", projectId), organization); | |||
| var ws = await AddOrganization(GetUrl("/api/qualityprofiles/search?project={0}", projectId), organization); | |||
There was a problem hiding this comment.
will this work with the current LTS 7.9?
There was a problem hiding this comment.
project param is here since 6.7
| @@ -83,15 +83,15 @@ public void Ctor_NullLogger_Throws() | |||
| [TestMethod] | |||
| public void LogWSOnError() | |||
| { | |||
| this.downloader.Pages["http://myhost:222/api/qualityprofiles/search?projectKey=foo+bar"] = "trash"; | |||
| this.downloader.Pages["http://myhost:222/api/qualityprofiles/search?project=foo+bar"] = "trash"; | |||
There was a problem hiding this comment.
are these tests on a SQ instance? can you add tests with the latest SQ and the LTS SQ?
There was a problem hiding this comment.
No, these are unit test, agnostic from SQ version.
There was a problem hiding this comment.
Are there any integration tests with SQ?
If there are none, it means it would rely on sonar-dotnet ITs for detecting problems, which is under optimal (would happen after a new release)
Could you add ITs with SQ in this sprint, to ensure quick feedback ?
@duncanp-sonar as answered to Andrei, this param is supported since the version 6.7 of SQ. I don't think that this is a "huge" breaking change then. |
@andrei-epure-sonarsource we do have ones, already (located in "its" folder on this repo) |
That depends on whether you are one of the users affected ;-) I agree the number of users affected should be small; the likelihood of those users having problems is high and the impact on those users is high. Regardless, it doesn't mean we shouldn't make breaking changes but we should still try to provide a good UX. I don't think there's anything that can be done in this ticket, though. I've created #904 as a suggested improvement to address this class of problem. |
mickael-caro-sonarsource
force-pushed
the
mc/804
branch
from
0ea8252 to
58c3c63
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
* Moved from WebClient to HttpClient. (#841) * Split TFS-specific code in a separate exe (#896) * Update license headers (#895) * Detect and take VsTestToolsInstallerInstalledToolLocation environment variable to set CodeCoverage.exe path. (#903) * Remove/Changed SonarQube-only log messages to be more generic with So… (#898) * Remove/Changed SonarQube-only log messages to be more generic with SonarCloud. * Fail fast if sonar.organization is provided in SonarQube.Analysis.xml (#901) * Changed deprecated projectKey parameters while fetching Quality Profile (#899) * Added .NET 5 flavor for the Scanner (#884) * Drop support for single-valued "sonar.cs.roslyn.reportFilePath" and "… (#902) * Drop support for single-valued "sonar.cs.roslyn.reportFilePath" and "sonar.cs.analyzer.projectOutPath" (and VB.Net siblings) * Add support for pure .NET Core Projects (#887) * If a ProjectGuid has not been found in either the csproj or the solution, generate a random one. * Fail fast in case the server license is invalid. (#907) * fixed bug - skipped shared across projects files (#876) * Update documentation (Proxy, .NET Core NET Version, added back conditional blocks for SQ/SC (#905) * Add a warning and clearer message in case of falling back to SonarOutputDir projectbBaseDir. (#908) * Add build configuration to TFS build
fixes #804