-
Notifications
You must be signed in to change notification settings - Fork 86
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 typos; Add concat-like Metadata.AddMetadata(Metadata)
; Add in-line docs for AvaloniaCompilationAssemblyProvider
usage
#458
Conversation
Public API members have backwards-compatibility maintained via aliases-like methods and properties ('=>' to correct definition).
Public API members receive aliases. Misnamed methods simply call correctly-named definitions and are aggressively-inlined.
…blyProvider usage
@@ -27,7 +27,9 @@ public class ProjectOutputInfo | |||
/// </summary> | |||
public string RuntimeIdentifier { get; } | |||
|
|||
public string TargetPlatfromIdentifier { get; } | |||
[Obsolete($"Member name mispelled. Use {nameof(TargetPlatformIdentifier)} instead.")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not part of the public API, do we need to keep old members?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used the following rules while refactoring:
- Private or local? Rename.
- If
InternalsVisibleTo
, a new symbol with correct spelling is added to the Internal API. Else, symbols are renamed. - Public API receives a new symbol with the correct spelling.
The property you marked for review appears to be part of the public API. Is AvaloniaVS.Models
somehow hidden from the public API? Should the property be treated as Internal?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AvaloniaVS is not a library, these APIs are used only in the extension itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood. I'll update references in the extension projects and remove the old symbols.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
TODO
Obsolete
symbols from AvaloniaVS.SharedAvaloniaVS.Shared\Models\ProjectOutputInfo.cs(30,10): Error CS0246: The type or namespace name 'ObsoleteAttribute' could not be found (are you missing a using directive or an assembly reference?)
AvaloniaVS.Shared\Models\ProjectOutputInfo.cs(30,10): Error CS0246: The type or namespace name 'Obsolete' could not be found (are you missing a using directive or an assembly reference?)
Features/Enhancements
Avalonia.Ide.CompletionEngine.Metadata.AddMetadata(Metadata metadata)
to merge dictionaries from one Metadata into another. Keys are only added if they are not already present. Existing keys' values remain unchanged.This can be used to add metadata from various build contexts e.g. TargetFrameworks (build for multiple frameworks) and if-else compiler statements. Some TODO comments indicate potential improvements. How do we indicate which context a metadatum came from?
Miscellaneous Changes
AvaloniaCompilationAssemblyProvider
ctor
andGetAssemblies()
. The API's usage misunderstood in AvaloniaVSCode's AvaloniaLanguageServer, causing builds to fail.AvaloniaVS.Shared
andAvalonia.Ide.CompletionEngine
corrected where understoodObsolete
annotation. Method aliases are aggressively in-lined.