Skip to content

Conversation

@DavidThielen
Copy link
Contributor

@DavidThielen DavidThielen commented Nov 6, 2023

Pull request description

PR meta checklist

  • Pull request is targeted at main branch for code
    or targeted at stable branch for documentation that is live on bunit.dev.
  • Pull request is linked to all related issues, if any.
  • I have read the CONTRIBUTING.md document.

Code PR specific checklist

  • My code follows the code style of this project and AspNetCore coding guidelines.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • I have updated the appropriate sub section in the CHANGELOG.md.
  • I have added, updated or removed tests to according to my changes.
    • All tests passed.

@linkdotnet
Copy link
Collaborator

Hey @DavidThielen,

thanks for the PR. I have 1, 2 suggestions

---

# Mocking Localization via `IStringLocalizer`
<p>This is very simple. First in your setup add the following:</p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can be bold - no need to call it very simple ;) it is a minimal and working one. I would rephrase that or even remove that sentence entirely.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed it.


# Mocking Localization via `IStringLocalizer`
<p>This is very simple. First in your setup add the following:</p>
<code>TestContext.Services.AddLocalization();</code>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Our "pattern" is to have whole blogs of code like
```csharp
TestContext.Services.AddLocalization();
```

But I would go a step further:
A minimal example of the production code enriched by the test-code would be ideal. But that is my opinion. Let's hear if @egil has a different stance here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did as you suggested. It doesn't show as code when I view it, but I'm assuming that something processes it when pushed up.

implemented suggestions
@DavidThielen
Copy link
Contributor Author

DavidThielen commented Nov 6, 2023 via email


<p>This is very simple. First in your setup add the following:</p>
<code>TestContext.Services.AddLocalization();</code>
<p>There are just two steps. First in your setup add the following:</p>
Copy link
Collaborator

@linkdotnet linkdotnet Nov 6, 2023

Choose a reason for hiding this comment

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

Another small amendment:
Bunit documentation "style" doesn't use "you" or "your".
Just things like:

Suggested change
<p>There are just two steps. First in your setup add the following:</p>
There are just two steps. First in the setup add the following:

And so on. There is also no need for <p> and stuff.

```csharp
TestContext.Services.AddLocalization();
```
<p>Then in your test code, when you need the localized string to compare, you write the following:</p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<p>Then in your test code, when you need the localized string to compare, you write the following:</p>
In the test code, when there is a need for the localized string to compare, the following is written:

```csharp
var localizer = ctx.Services.GetService<IStringLocalizer<SharedStrings>>();
```
<p>Where SharedStrings.cs (you can name this anything you want) that has the resource files such as `SharedStrings.en.resx`</p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<p>Where SharedStrings.cs (you can name this anything you want) that has the resource files such as `SharedStrings.en.resx`</p>
Where `SharedStrings.cs` (which may be named as desired) contains the resource files such as `SharedStrings.en.resx`.

<code>var localizer = ctx.Services.GetService<IStringLocalizer<SharedStrings>>();</code>
```csharp
var localizer = ctx.Services.GetService<IStringLocalizer<SharedStrings>>();
```
Copy link
Collaborator

Choose a reason for hiding this comment

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

Some smaller whitespaces, that can be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

please make any changes you want.

@DavidThielen
Copy link
Contributor Author

DavidThielen commented Nov 6, 2023 via email

@egil
Copy link
Member

egil commented Nov 13, 2023

Sorry been travelling for a conference. Thanks for this, it looks alright.

@egil egil enabled auto-merge (squash) November 13, 2023 12:05
@egil egil disabled auto-merge November 13, 2023 12:05
@egil egil merged commit 1bb7f47 into bUnit-dev:stable Nov 13, 2023
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