zarusz / SlimMessageBus

Lightweight message bus interface for .NET (pub/sub and request-response) with transport plugins for popular message brokers.
Apache License 2.0
467 stars 78 forks source link

Fix sonar warnings #307

Closed dundich closed 3 hours ago

dundich commented 4 days ago

Signed-off-by: krivchenko_kv kvkrivchenko@activebc.ru

zarusz commented 4 days ago

@dundich looks good. Can you please squash all your commits into one? Then I can accept the PR. Thanks for your contribution!

dundich commented 2 days ago

fixed: all commits in one... but he didn't pass the integration test, I don't know :(

zarusz commented 2 days ago

@dundich I believe the branch you want to merge from your fork does not have the recent changes needed to bring Kafka integration tests back on track (build.yaml).

From this view, your branch is 3 commits behind: https://github.com/dundich/SlimMessageBus/compare/sonar-fix...zarusz%3ASlimMessageBus%3Amaster

Please pull the latest changes from this (original) repo into your fork. Then rebase your branch aginst the latest master.

dundich commented 1 day ago

Thanks for the tips. Done!

dundich commented 13 hours ago

Unused local variables should be removed ???

https://github.com/zarusz/SlimMessageBus/blob/bd33379966f710a13b5f4fc15ac084b78ab3eb08/src/SlimMessageBus.Host/DependencyResolver/ServiceCollectionExtensions.cs#L76

ps. I didn't remove it because I'm afraid of the side effect.

sonarcloud[bot] commented 13 hours ago

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
81.6% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

dundich commented 13 hours ago

Constructors should only call non-overridable methods

https://github.com/zarusz/SlimMessageBus/blob/bd33379966f710a13b5f4fc15ac084b78ab3eb08/src/SlimMessageBus.Host/Hybrid/HybridMessageBus.cs#L40C11-L40C12

https://rules.sonarsource.com/csharp/RSPEC-1699/

    protected virtual MessageBusBase BuildBus(MessageBusBuilder builder)

ps. Maybe - sealed class?

same warn...

public class JsonMessageSerializer : IMessageSerializer
{
    public JsonSerializerOptions Options { get; set; }

    public JsonMessageSerializer(JsonSerializerOptions options = null)
    {
        Options = options ?? CreateDefaultOptions();
    }

    public virtual JsonSerializerOptions CreateDefaultOptions()  // <==  !!??
    {
dundich commented 13 hours ago

That's it. I'm done. :)

zarusz commented 3 hours ago

Unused local variables should be removed ???

https://github.com/zarusz/SlimMessageBus/blob/bd33379966f710a13b5f4fc15ac084b78ab3eb08/src/SlimMessageBus.Host/DependencyResolver/ServiceCollectionExtensions.cs#L76

ps. I didn't remove it because I'm afraid of the side effect.

It should be safe to remove - I will do it.

Thanks for contribution @dundich !