Style Checking EventHubs #7635
Style Checking EventHubs #7635
Conversation
chidozieononiwu
requested review from
jsquire,
kinelski and
serkantkaraca
as code owners
|
/azp run net - eventhub - ci |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
LGTM. There were too many files to go through each line-by-line, but the random sampling that I did looked ok.
There's one part that didn't match my expectations, with respect to our treatment of readonly and static readonly members, which are intended to be "constants that you have to assign at initialization". I would expect those to follow the const rules rather than the class/static member ones. I'm hoping that @AlexGhiondea can weigh in there.
I want to scream at the sky to give me back my vars, but I realize that I've already lost that debate and instead will look across the verbose landscape of code left smoldering in the wake of reformatting and lament my fate.
| @@ -21,13 +21,13 @@ namespace Azure.Messaging.EventHubs.CheckpointStore.Blob | |||
| public sealed class BlobPartitionManager : PartitionManager | |||
| { | |||
| /// <summary>A regular expression used to capture strings enclosed in double quotes.</summary> | |||
| private static readonly Regex DoubleQuotesExpression = new Regex("\"(.*)\"", RegexOptions.Compiled); | |||
| private static readonly Regex s_doubleQuotesExpression = new Regex("\"(.*)\"", RegexOptions.Compiled); | |||
There was a problem hiding this comment.
I could have sworn that our guidance was to treat static readonly in the same manner as const and only use the s_ prefix for things that were variable. No?
@AlexGhiondea - Any chance that you can weigh in with your thoughts here?
chidozieononiwu
force-pushed
the
StyleCheckingEventHubs
branch
from
a382d85 to
22c1e9f
Compare
chidozieononiwu
force-pushed
the
StyleCheckingEventHubs
branch
from
22c1e9f to
fe400a7
Compare
No description provided.