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

refactor: 🔥 Removed initializeMemberFromCtor command #93

Merged
merged 3 commits into from
Jul 2, 2023

Conversation

KreativJos
Copy link
Owner

initializeMemberFromCtor is provided by the C# extension (omnisharp)

#75

@KreativJos
Copy link
Owner Author

Don't know about versioning with this, but I do not think it really matters. I don't expect anyone to have an issue with this, because the feature is included in the default C# extension.

@bard83
Copy link
Collaborator

bard83 commented Apr 24, 2022

It won't matter exactly for the reason you mentioned. Probably redundancy should be considered as a bug. Omnisharp has an higher priority/consideration compared to the extension.

@bard83
Copy link
Collaborator

bard83 commented Jun 13, 2022

Don't know about versioning with this, but I do not think it really matters. I don't expect anyone to have an issue with this, because the feature is included in the default C# extension.

I've miss understood on the first round, I guess that your retro-compatibility concern is more related to the following situation:
the user does not have the specific C# extension version - where the initialization feature for ctor exists - but it gets this change -> the user might loose the feature because none provides it.
Possible approaches:

  • don't care about it: the user will downgrade the version in case (risk is that the user might uninstall the extension because it gets disappointed with the decision and also could struggle with future updates).
  • opt-in configuration flag: show the feature in case the configuration is flagged (currently omnisharp is the de facto standard c# extension for vs code and the choice to remove the feature relies on it, but what will happen in case it won't be anymore in the future?).
  • show the feature smartly: check whether omnisharp provides it (check the omnisharp version? don't know...) or does not.

To me the second could be the correct decision, don't know if it's technically doable.

KreativJos and others added 3 commits June 10, 2023 15:30
initializeMemberFromCtor is provided by the C# extension (omnisharp)
- fixed minor bug: regex did not grab all properties
@bard83 bard83 force-pushed the issue/75-removeInitializeMemberFromCtor branch from b72a3e5 to f262df8 Compare July 2, 2023 21:12
@bard83 bard83 marked this pull request as ready for review July 2, 2023 21:15
@bard83 bard83 merged commit fd23720 into master Jul 2, 2023
3 checks passed
@bard83 bard83 deleted the issue/75-removeInitializeMemberFromCtor branch July 2, 2023 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants