Skip to content

An assortment of minor docs and spelling fixes#2733

Merged
holytshirt merged 5 commits into
BrighterCommand:masterfrom
slang25:xmldocs-and-spelling
Feb 13, 2024
Merged

An assortment of minor docs and spelling fixes#2733
holytshirt merged 5 commits into
BrighterCommand:masterfrom
slang25:xmldocs-and-spelling

Conversation

@slang25

@slang25 slang25 commented Jul 14, 2023

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread samples/ASBTaskQueue/Greetings/Adaptors/Services/UnitOfWork.cs
public DynamoDBEntry ToEntry(object value)
{
var uuid = (Guid)value;
if (uuid == null)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hard cast will throw and uuid will never be null. Changing for a version that throws the intended exception

var uuid = (Guid)value;
if (uuid == null)
throw new InvalidOperationException(
$"Supplied type was of type {value.GetType().Name} not Accounts.Application.CardDetails");

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not sure where Accounts.Application.CardDetails has come from 🤪

.SetPartitionsRevokedHandler((consumer, list) =>
{
_consumer.Commit(list);
consumer.Commit(list);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Presumably this was incorrectly capturing the field in the class rather than using the provided consumer

Comment thread src/Paramore.Brighter.Outbox.DynamoDB/DynamoDbOutbox.cs Outdated
Comment thread src/Paramore.Brighter.Outbox.DynamoDB/DynamoDbOutbox.cs
Comment thread src/Paramore.Brighter/MessageHeader.cs
/// <value>The no of peformers.</value>
public int NoOfPeformers { get; private set; }
/// <value>The no of performers.</value>
public int NoOfPerformers { get; private set; }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Renamed this property and marked the misspelt version as obsolete

@iancooper

Copy link
Copy Markdown
Member

@slang25 This looks very useful. Let us know when you are done.

@iancooper

Copy link
Copy Markdown
Member

@slang25 How is this going?

@holytshirt

Copy link
Copy Markdown
Member

@iancooper I removed the merge and rebased @slang25 change on top of latest master, so we can now pull these changes in easily

@holytshirt holytshirt marked this pull request as ready for review February 12, 2024 22:38
@holytshirt holytshirt requested a review from iancooper February 12, 2024 22:48
@holytshirt

Copy link
Copy Markdown
Member

@iancooper can you give this another review before I merge it please

$"Topic: {tpo.Topic} Partition: {tpo.Partition.Value} Offset: {tpo.Offset.Value}");
var offsetAsString = string.Join(Environment.NewLine, offsets);
s_logger.LogInformation($"Sweeping offsets: {Environment.NewLine} {{Offset}}", offsetAsString);
s_logger.LogInformation("Sweeping offsets: {0} {Offset}", Environment.NewLine, offsetAsString);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was deliberate, as in this case I think you do want Environment.NewLine in the message argument, and not captured as a named parameter with name 0. Although the string interpolation will probably trigger analyzers.

Comment thread README.md
| Documentation | [Technical Documentation](https://brightercommand.gitbook.io/paramore-brighter-documentation/) |
| Source |https://github.com/BrighterCommand/Brighter |
| Keywords |task queue, job queue, asynchronous, async, rabbitmq, amqp, sqs, sns, kakfa, redis, c#, command, command dispatcher, command processor, queue, distributed |
| Keywords |task queue, job queue, asynchronous, async, rabbitmq, amqp, sqs, sns, kafka, redis, c#, command, command dispatcher, command processor, queue, distributed |

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I spell kafka like this all the time, and looking at this recent PR title, old habits die hard 😄
#3014

@holytshirt holytshirt merged commit 1828350 into BrighterCommand:master Feb 13, 2024
@holytshirt

Copy link
Copy Markdown
Member

Thanks @slang25 sorry it took so long, we have been working on a new version with some big changes

@slang25

slang25 commented Feb 13, 2024

Copy link
Copy Markdown
Contributor Author

No worries, I had left it in draft forever so it was my bad

DevJonny pushed a commit to DevJonny/Brighter that referenced this pull request Feb 28, 2026
* An assortment of minor docs and spelling fixes

* fix(dynamo): clean merge issue and interface updates

* fix(kafka): don't use string interpolation in log messages

* chore(readme): fix kafka spelling

* Revert redundant change

---------

Co-authored-by: Toby Henderson <hendersont@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants