Skip to content
This repository has been archived by the owner on Dec 28, 2017. It is now read-only.

Using 'nameof' operator instead of magic string #8

Closed
wants to merge 1 commit into from

Conversation

hishamco
Copy link
Contributor

No description provided.

@dnfclas
Copy link

dnfclas commented Jun 11, 2015

Hi @hishamco, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

@hishamco
Copy link
Contributor Author

/cc @Eilon

@@ -37,7 +37,7 @@ public void RetryRetriesActionOnStandardException()
{
++called;
if (called < 2)
throw new Exception("Error");
Copy link
Contributor

Choose a reason for hiding this comment

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

This is wrong. This Exception ctor takes a message as the parameter and not parameter name. This change does not add any value and actually makes the code more confusing. Same applies to changes below.

(From what I see a good change in this file would be to remove ref from the ref int called parameter on the RetryHelper method - it does not seem to be changed inside the method and therefore does not have to be ref - the tests still must build and pass after the change).

@hishamco
Copy link
Contributor Author

@Eilon I update the PR to use nameof and left the NotNull while I have seen @pranavkm applying a script to throw ArgumentNullException instead

@hishamco hishamco changed the title Using [NotNull] and 'nameof' operator Using 'nameof' operator Oct 20, 2015
@hishamco hishamco changed the title Using 'nameof' operator Using 'nameof' operator instead of magic string Oct 20, 2015
@hishamco
Copy link
Contributor Author

@Eilon can you have a look and merge if it's fine

1 similar comment
@hishamco
Copy link
Contributor Author

@Eilon can you have a look and merge if it's fine

@pranavkm
Copy link
Contributor

We're no longer building this repo.

@pranavkm pranavkm closed this Jun 30, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants