CallJsExample7 - catch-throw removal #351
Conversation
|
Thanks @hakenr ... One of the product unit engineers will need to look at this. They sent me this example for publication, and I assume they set it up this way with some kind of intent. They obviously didn't tell me to explain to readers why they just rethrow there, or I would've stated something in the article about it. They're all busy at the moment finishing up for RC2. I'll contact Artak offline and ask him to add this to his list of things to address. |
|
I found the issue ... it was Javier ... Since he definitely doesn't do random things 😆, we definitely want him to take a look. Artak will let him know ... or he'll let me know that it's ok to contact him. My understanding is that they're very busy right now, so it's best to hold off on anything that isn't a bug right now. |
|
There are a few rare cases where such a rethrowing construct might make sense, but in that case, it's worth adding a comment with an explanation. |
8.0/BlazorSample_BlazorWebApp/Components/Pages/CallJs7.razor.cs
Outdated
Show resolved
Hide resolved
Oops, sorry, my Visual Studio's Format Document removed them. @guardrex Are these spaces specific to the MS Learn platform? I don't recall ever seeing code formatted this way with intentional spaces at the end of a line (even if the statement continues on the next line). |
|
In most cases, the broken lines are present to conform code examples to the 85 char limit for display in articles. If a dev takes some code with cut-'n-paste, they may want to re-assemble the lines. If the space isn't there, they must manually add it back. I leave them there when I break lines as a minor convenience for those devs. |
|
Indeed! As I work through NIT code updates, I can see that VS is always fighting my effort to make recombining lines just a hair quicker by removing the space in these cases. I'm not going to seek to adjust editor config because there are many cases where I do want to end the line without a space. I'll stick with my convention, but it is a super tiny NIT. |
|
@guardrex This seems to be mergeable? I will create a pull-commit in |
|
(@guardrex The pull-commit to |
|
Yes, we're just waiting on Javier to look. If we don't hear from him here by RC2 release next week, I'll ping him to take a look at this on Wednesday ........ IF I'm still alive on Wednesday! Hurricane Milton might try to kill me 💀😨 next week (I live on the "Space Coast" in Florida). |
|
Oh, sorry, I thought someone from PU had already seen it. No problem. |
This
catchstatement just rethrows the exception and doesn't add any value to the sample.(Maybe I would also update the other try-catch in the
Dispose()method. The usual convention is to add a// NOOPcomment in an emptycatchstatement to explicitly indicate the intent to ignore the exception.)If you approve this PR, I will create a PR to aspnetdoc repo to pull the updated sample there.
cc @guardrex