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

[plugin-kafka] Aligh syntax with the official documentation #3682

Merged
merged 1 commit into from
Feb 28, 2023

Conversation

uarlouski
Copy link
Member

No description provided.

@uarlouski uarlouski requested a review from a team as a code owner February 27, 2023 16:43
@codecov
Copy link

codecov bot commented Feb 27, 2023

Codecov Report

Merging #3682 (6dca766) into master (bb9562e) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             master    #3682    +/-   ##
==========================================
  Coverage     96.97%   96.97%            
+ Complexity     6309     6092   -217     
==========================================
  Files           860      860            
  Lines         17441    17447     +6     
  Branches       1136     1136            
==========================================
+ Hits          16914    16920     +6     
  Misses          422      422            
  Partials        105      105            
Impacted Files Coverage Δ
.../main/java/org/vividus/steps/kafka/KafkaSteps.java 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

docs/modules/plugins/pages/plugin-kafka.adoc Outdated Show resolved Hide resolved
docs/modules/plugins/pages/plugin-kafka.adoc Outdated Show resolved Hide resolved
docs/modules/plugins/pages/plugin-kafka.adoc Outdated Show resolved Hide resolved

.Deprecated syntax
Copy link
Member

Choose a reason for hiding this comment

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

i think it is better to put the new syntax before the old one

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree, but I copied this from another place, so probably it makes sense to do it in a separate commit

Comment on lines 214 to 215
() -> getEventsBy(consumerKey).size(),
countMatcher::matches);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
() -> getEventsBy(consumerKey).size(),
countMatcher::matches);
() -> getEventsBy(consumerKey).size(), countMatcher::matches);

Copy link
Member

Choose a reason for hiding this comment

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

optional, just looks strange to me :)

@ikalinin1 ikalinin1 self-requested a review February 28, 2023 16:07
@sonarcloud
Copy link

sonarcloud bot commented Feb 28, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@valfirst valfirst merged commit 2ea5d36 into master Feb 28, 2023
@valfirst valfirst deleted the kafka-syntax-alignment branch February 28, 2023 17:34
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.

3 participants