Skip to content

Remove unused parameters: analyzer and fix#207

Merged
carloscds merged 2 commits intocode-cracker:masterfrom
giggio:unused-parameters
Jan 14, 2015
Merged

Remove unused parameters: analyzer and fix#207
carloscds merged 2 commits intocode-cracker:masterfrom
giggio:unused-parameters

Conversation

@giggio
Copy link
Copy Markdown
Member

@giggio giggio commented Jan 9, 2015

Fixes #24.

image

@giggio
Copy link
Copy Markdown
Member Author

giggio commented Jan 9, 2015

@viniciushana
Copy link
Copy Markdown
Contributor

Looks good!

@carloscds
Copy link
Copy Markdown

@viniciushana Are you working in this PR ?

@viniciushana
Copy link
Copy Markdown
Contributor

Not really, just did a quick glance over the changes.

@carloscds
Copy link
Copy Markdown

ok. I will go analyze it.

@carloscds carloscds self-assigned this Jan 9, 2015
@carloscds
Copy link
Copy Markdown

@giggio If I have a call to method that has unused parameter and I apply this, my code broken:

 static void Main(string[] args)
        {
            foo(10, 20);

        }

        public static void foo(int a, int b)
        {
            int c = a;

        }

become this:

 static void Main(string[] args)
        {
            foo(10, 20); // error here

        }

        public static void foo(int a)
        {
            int c = a;

        }

Do you expect to check this ?

@viniciushana
Copy link
Copy Markdown
Contributor

And then again we reach a point where we need to fix references done throughout the whole codebase :(

@giggio
Copy link
Copy Markdown
Member Author

giggio commented Jan 10, 2015

@carloscds another one of those problems of referenced code, that may be anywhere on the solution. Good catch, Carlos! But I don't know what to do... I think, from what we discussed earlier this week on our meeting, that we can make the change and let it break, but I am not sure... what do you think? @viniciushana @ElemarJR what do you think?

@carloscds
Copy link
Copy Markdown

@giggio It´s same case of privateset, maybe change to hidden and not warning.

@giggio
Copy link
Copy Markdown
Member Author

giggio commented Jan 11, 2015

@carloscds I think I found a solution. As we are going to remove the parameter, I can do that on the code fix, and that can be async. Hold on, I will fix it, I will send an update.

@giggio giggio mentioned this pull request Jan 13, 2015
@giggio giggio force-pushed the unused-parameters branch from 2bc5c64 to 33ec88a Compare January 14, 2015 03:51
@giggio
Copy link
Copy Markdown
Member Author

giggio commented Jan 14, 2015

@carloscds this was fixed. Reference is now updated. Beautiful! 🚴
@ElemarJR Check out the code fix, I think you will like it.

@giggio
Copy link
Copy Markdown
Member Author

giggio commented Jan 14, 2015

@carloscds
Copy link
Copy Markdown

@giggio Cool.

@carloscds
Copy link
Copy Markdown

@giggio Can I merge it ?

@giggio
Copy link
Copy Markdown
Member Author

giggio commented Jan 14, 2015

@carloscds Yes, you can

carloscds pushed a commit that referenced this pull request Jan 14, 2015
Remove unused parameters: analyzer and fix
@carloscds carloscds merged commit bdb8299 into code-cracker:master Jan 14, 2015
@giggio giggio deleted the unused-parameters branch January 14, 2015 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove unused parameters

3 participants