Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix parameters position calculation #2905

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

asvishnyakov
Copy link

@RicoSuter
Copy link
Owner

I think this is a breaking change and cannot be merged - also tests are failing.

@asvishnyakov
Copy link
Author

asvishnyakov commented Jul 2, 2020

@RicoSuter I don't think failed tests are related to PR changes. The error is about missed nuget packages. Probably we need to run checks for PR again. Please check your tests and CI.

Yes, it's breaking changes, but without that fix Position parameter will work only if you have GenerateOptionalParameters option enabled, which is not useful, because optional parameters are always last, so you never can have your parameters correctly ordered. We probably need introduce new option for that case

@asvishnyakov
Copy link
Author

asvishnyakov commented Aug 13, 2020

@RicoSuter As I said tests failed not because of these changes. After merging latest changes from master (and CI run caused by that), everything passed. Please review and merge is possible or ask me for changes

@RicoSuter
Copy link
Owner

I'm very afraid to break many people as this might reorder parameters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants